On 2024-05-17 02:06 +0200, Michael Paquier wrote:
> On Thu, May 16, 2024 at 08:41:11AM +0200, Peter Eisentraut wrote:
> > On this specific patch, maybe reword "parameter too large" to "parameter
> > number too large".
>
> WFM here.
Done in v3.
I noticed this compiler warning with my previous patch:
scan.l:997:41: warning: ISO C90 forbids mixed declarations and code
[-Wdeclaration-after-statement]
997 | ErrorSaveContext escontext
= {T_ErrorSaveContext};
| ^~~~~~~~~~~~~~~~
I thought that I had to factor this out into a function similar to
process_integer_literal (which also uses ErrorSaveContext). But moving
that declaration to the start of the {param} action was enough in the
end.
While trying out the refactoring, I noticed two small things that can be
fixed as well in scan.l:
* Prototype and definition of addunicode do not match. The prototype
uses yyscan_t while the definition uses core_yyscan_t.
* Parameter base of process_integer_literal is unused.
But those should be one a separate thread, right, even for minor fixes?
--
Erik
>From d3ad2971dcb8ab15fafe1a5c69a15e5994eac76d Mon Sep 17 00:00:00 2001
From: Erik Wienhold <e...@ewie.name>
Date: Tue, 14 May 2024 14:12:11 +0200
Subject: [PATCH v3 1/3] Fix overflow in parsing of positional parameter
Replace atol with pg_strtoint32_safe in the backend parser and with
strtoint in ECPG to reject overflows when parsing the number of a
positional parameter. With atol from glibc, parameters $2147483648 and
$4294967297 turn into $-2147483648 and $1, respectively.
---
src/backend/parser/scan.l | 8 +++++++-
src/interfaces/ecpg/preproc/pgc.l | 5 ++++-
src/test/regress/expected/numerology.out | 4 ++++
src/test/regress/sql/numerology.sql | 1 +
4 files changed, 16 insertions(+), 2 deletions(-)
diff --git a/src/backend/parser/scan.l b/src/backend/parser/scan.l
index 9b33fb8d72..436cc64f07 100644
--- a/src/backend/parser/scan.l
+++ b/src/backend/parser/scan.l
@@ -992,8 +992,14 @@ other .
}
{param} {
+ ErrorSaveContext escontext = {T_ErrorSaveContext};
+ int32 val;
+
SET_YYLLOC();
- yylval->ival = atol(yytext + 1);
+ val = pg_strtoint32_safe(yytext + 1, (Node *) &escontext);
+ if (escontext.error_occurred)
+ yyerror("parameter number too large");
+ yylval->ival = val;
return PARAM;
}
{param_junk} {
diff --git a/src/interfaces/ecpg/preproc/pgc.l b/src/interfaces/ecpg/preproc/pgc.l
index d117cafce6..7122375d72 100644
--- a/src/interfaces/ecpg/preproc/pgc.l
+++ b/src/interfaces/ecpg/preproc/pgc.l
@@ -938,7 +938,10 @@ cppline {space}*#([^i][A-Za-z]*|{if}|{ifdef}|{ifndef}|{import})((\/\*[^*/]*\*+
}
{param} {
- base_yylval.ival = atol(yytext+1);
+ errno = 0;
+ base_yylval.ival = strtoint(yytext+1, NULL, 10);
+ if (errno == ERANGE)
+ mmfatal(PARSE_ERROR, "parameter number too large");
return PARAM;
}
{param_junk} {
diff --git a/src/test/regress/expected/numerology.out b/src/test/regress/expected/numerology.out
index c8196d2c85..5bac05c3b3 100644
--- a/src/test/regress/expected/numerology.out
+++ b/src/test/regress/expected/numerology.out
@@ -206,6 +206,10 @@ PREPARE p1 AS SELECT $1a;
ERROR: trailing junk after parameter at or near "$1a"
LINE 1: PREPARE p1 AS SELECT $1a;
^
+PREPARE p1 AS SELECT $2147483648;
+ERROR: parameter number too large at or near "$2147483648"
+LINE 1: PREPARE p1 AS SELECT $2147483648;
+ ^
SELECT 0b;
ERROR: invalid binary integer at or near "0b"
LINE 1: SELECT 0b;
diff --git a/src/test/regress/sql/numerology.sql b/src/test/regress/sql/numerology.sql
index 3f0ec34ecf..6722a09c5f 100644
--- a/src/test/regress/sql/numerology.sql
+++ b/src/test/regress/sql/numerology.sql
@@ -52,6 +52,7 @@ SELECT 0.0e1a;
SELECT 0.0e;
SELECT 0.0e+a;
PREPARE p1 AS SELECT $1a;
+PREPARE p1 AS SELECT $2147483648;
SELECT 0b;
SELECT 1b;
--
2.45.1
