On 2024-05-14 16:40 +0200, Tom Lane wrote:
> Dean Rasheed <dean.a.rash...@gmail.com> writes:
> > On Tue, 14 May 2024 at 07:43, Michael Paquier <mich...@paquier.xyz> wrote:
> >> On Tue, May 14, 2024 at 05:18:24AM +0200, Erik Wienhold wrote:
> >>> Parameter $1_2 is taken as $1 because in rule {param} in scan.l we get
> >>> the parameter number with atol which stops at the underscore. That's a
> >>> regression in faff8f8e47f. Before that commit, $1_2 resulted in
> >>> "ERROR: trailing junk after parameter".
>
> > I'm sure that this wasn't intentional -- I think we just failed to
> > notice that "param" also uses "decinteger" in the scanner. Taking a
> > quick look, there don't appear to be any other uses of "decinteger",
> > so at least it only affects params.
>
> > Unless the spec explicitly says otherwise, I agree that we should
> > reject this, as we used to do, and add a comment saying that it's
> > intentionally not supported. I can't believe it would ever be useful,
> > and the current behaviour is clearly broken.
>
> +1, let's put this back the way it was.
I split the change in two independent patches:
Patch 0001 changes rules param and param_junk to only accept digits 0-9.
Patch 0002 replaces atol with pg_strtoint32_safe in the backend parser
and strtoint in ECPG. This fixes overflows like:
=> PREPARE p1 AS SELECT $4294967297; -- same as $1
PREPARE
=> EXECUTE p1 (123);
?column?
----------
123
(1 row)
=> PREPARE p2 AS SELECT $2147483648;
ERROR: there is no parameter $-2147483648
LINE 1: PREPARE p2 AS SELECT $2147483648;
It now returns this error:
=> PREPARE p1 AS SELECT $4294967297;
ERROR: parameter too large at or near $4294967297
=> PREPARE p2 AS SELECT $2147483648;
ERROR: parameter too large at or near $2147483648
--
Erik
>From a3a3d845bbc60cb5ff1bc510205c84ab3bdeddbf Mon Sep 17 00:00:00 2001
From: Erik Wienhold <e...@ewie.name>
Date: Tue, 14 May 2024 14:18:59 +0200
Subject: [PATCH v2 1/2] Forbid underscore in positional parameters
Underscores were added to numeric literals in faff8f8e47. In an
oversight, this change also affected the positional parameters rule
which use the same production for its digits. Underscores are not
necessary for positional parameters, so revert that rule to its old form
that only accepts digits 0-9.
---
src/backend/parser/scan.l | 5 +++--
src/fe_utils/psqlscan.l | 5 +++--
src/interfaces/ecpg/preproc/pgc.l | 5 +++--
src/test/regress/expected/numerology.out | 4 ++++
src/test/regress/sql/numerology.sql | 1 +
5 files changed, 14 insertions(+), 6 deletions(-)
diff --git a/src/backend/parser/scan.l b/src/backend/parser/scan.l
index 5eaadf53b2..b499975e9c 100644
--- a/src/backend/parser/scan.l
+++ b/src/backend/parser/scan.l
@@ -419,8 +419,9 @@ bininteger_junk {bininteger}{ident_start}
numeric_junk {numeric}{ident_start}
real_junk {real}{ident_start}
-param \${decinteger}
-param_junk \${decinteger}{ident_start}
+/* Positional parameters don't accept underscores. */
+param \${decdigit}+
+param_junk \${decdigit}+{ident_start}
other .
diff --git a/src/fe_utils/psqlscan.l b/src/fe_utils/psqlscan.l
index c9df0594fd..c6d02439ab 100644
--- a/src/fe_utils/psqlscan.l
+++ b/src/fe_utils/psqlscan.l
@@ -355,8 +355,9 @@ bininteger_junk {bininteger}{ident_start}
numeric_junk {numeric}{ident_start}
real_junk {real}{ident_start}
-param \${decinteger}
-param_junk \${decinteger}{ident_start}
+/* Positional parameters don't accept underscores. */
+param \${decdigit}+
+param_junk \${decdigit}+{ident_start}
/* psql-specific: characters allowed in variable names */
variable_char [A-Za-z\200-\377_0-9]
diff --git a/src/interfaces/ecpg/preproc/pgc.l b/src/interfaces/ecpg/preproc/pgc.l
index bcfbd0978b..d117cafce6 100644
--- a/src/interfaces/ecpg/preproc/pgc.l
+++ b/src/interfaces/ecpg/preproc/pgc.l
@@ -388,8 +388,9 @@ bininteger_junk {bininteger}{ident_start}
numeric_junk {numeric}{ident_start}
real_junk {real}{ident_start}
-param \${decinteger}
-param_junk \${decinteger}{ident_start}
+/* Positional parameters don't accept underscores. */
+param \${decdigit}+
+param_junk \${decdigit}+{ident_start}
/* special characters for other dbms */
/* we have to react differently in compat mode */
diff --git a/src/test/regress/expected/numerology.out b/src/test/regress/expected/numerology.out
index f662a5050a..c8196d2c85 100644
--- a/src/test/regress/expected/numerology.out
+++ b/src/test/regress/expected/numerology.out
@@ -330,6 +330,10 @@ SELECT 1_000.5e_1;
ERROR: trailing junk after numeric literal at or near "1_000.5e"
LINE 1: SELECT 1_000.5e_1;
^
+PREPARE p1 AS SELECT $0_1;
+ERROR: trailing junk after parameter at or near "$0_"
+LINE 1: PREPARE p1 AS SELECT $0_1;
+ ^
--
-- Test implicit type conversions
-- This fails for Postgres v6.1 (and earlier?)
diff --git a/src/test/regress/sql/numerology.sql b/src/test/regress/sql/numerology.sql
index 1941c58e68..3f0ec34ecf 100644
--- a/src/test/regress/sql/numerology.sql
+++ b/src/test/regress/sql/numerology.sql
@@ -88,6 +88,7 @@ SELECT 1_000._5;
SELECT 1_000.5_;
SELECT 1_000.5e_1;
+PREPARE p1 AS SELECT $0_1;
--
-- Test implicit type conversions
--
2.45.0
>From debc576256d53a2c4ca46c3ca117500f16998f7b Mon Sep 17 00:00:00 2001
From: Erik Wienhold <e...@ewie.name>
Date: Tue, 14 May 2024 14:12:11 +0200
Subject: [PATCH v2 2/2] 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 | 5 ++++-
src/interfaces/ecpg/preproc/pgc.l | 5 ++++-
src/test/regress/expected/numerology.out | 4 ++++
src/test/regress/sql/numerology.sql | 1 +
4 files changed, 13 insertions(+), 2 deletions(-)
diff --git a/src/backend/parser/scan.l b/src/backend/parser/scan.l
index b499975e9c..a4f6973950 100644
--- a/src/backend/parser/scan.l
+++ b/src/backend/parser/scan.l
@@ -993,7 +993,10 @@ other .
{param} {
SET_YYLLOC();
- yylval->ival = atol(yytext + 1);
+ ErrorSaveContext escontext = {T_ErrorSaveContext};
+ yylval->ival = pg_strtoint32_safe(yytext + 1, (Node *) &escontext);
+ if (escontext.error_occurred)
+ yyerror("parameter too large");
return PARAM;
}
{param_junk} {
diff --git a/src/interfaces/ecpg/preproc/pgc.l b/src/interfaces/ecpg/preproc/pgc.l
index d117cafce6..178686e413 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 too large");
return PARAM;
}
{param_junk} {
diff --git a/src/test/regress/expected/numerology.out b/src/test/regress/expected/numerology.out
index c8196d2c85..f9b7eb2764 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 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.0
