On 2024-05-19 07:00 +0200, Alexander Lakhin wrote: > I encountered anomalies that you address with this patch too. > And I can confirm that it fixes most cases, but there is another one: > SELECT $300000000 \bind 'foo' \g > ERROR: invalid memory alloc request size 1200000000 > > Maybe you would find this worth fixing as well.
Yes, that error message is not great. In variable_paramref_hook we check paramno > INT_MAX/sizeof(Oid) when in fact MaxAllocSize/sizeof(Oid) is the more appropriate limit to avoid that unspecific alloc size error. Fixed in v4 with a separate patch because it's unrelated to the param number parsing. But it fits nicely into the broader issue on the upper limit for param numbers. Note that $268435455 is still the largest possible param number ((2^30-1)/4) and that we just return a more user-friendly error message for params beyond that limit. -- Erik
>From 5382f725ac1ff99b5830e8ca04613467660afaa2 Mon Sep 17 00:00:00 2001 From: Erik Wienhold <e...@ewie.name> Date: Tue, 14 May 2024 14:12:11 +0200 Subject: [PATCH v4 1/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 | 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
>From 9ae52b4a1949a0498dc75aba8fdd72927e6bd93d Mon Sep 17 00:00:00 2001 From: Erik Wienhold <e...@ewie.name> Date: Sun, 19 May 2024 15:29:18 +0200 Subject: [PATCH v4 2/2] Limit max parameter number with MaxAllocSize MaxAllocSize puts an upper bound on the largest possible parameter number ($268435455). Use that limit instead of MAX_INT to report that no parameters exist beyond that point instead of reporting an error about the maximum allocation size being exceeded. --- src/backend/parser/parse_param.c | 3 ++- src/test/regress/expected/prepare.out | 5 +++++ src/test/regress/sql/prepare.sql | 3 +++ 3 files changed, 10 insertions(+), 1 deletion(-) diff --git a/src/backend/parser/parse_param.c b/src/backend/parser/parse_param.c index dbf1a7dff0..b617591ef6 100644 --- a/src/backend/parser/parse_param.c +++ b/src/backend/parser/parse_param.c @@ -31,6 +31,7 @@ #include "parser/parse_param.h" #include "utils/builtins.h" #include "utils/lsyscache.h" +#include "utils/memutils.h" typedef struct FixedParamState @@ -136,7 +137,7 @@ variable_paramref_hook(ParseState *pstate, ParamRef *pref) Param *param; /* Check parameter number is in range */ - if (paramno <= 0 || paramno > INT_MAX / sizeof(Oid)) + if (paramno <= 0 || paramno > MaxAllocSize / sizeof(Oid)) ereport(ERROR, (errcode(ERRCODE_UNDEFINED_PARAMETER), errmsg("there is no parameter $%d", paramno), diff --git a/src/test/regress/expected/prepare.out b/src/test/regress/expected/prepare.out index 5815e17b39..853cbed248 100644 --- a/src/test/regress/expected/prepare.out +++ b/src/test/regress/expected/prepare.out @@ -184,6 +184,11 @@ SELECT name, statement, parameter_types, result_types FROM pg_prepared_statement | UPDATE tenk1 SET stringu1 = $2 WHERE unique1 = $1; | | (6 rows) +-- max parameter number and one above +PREPARE q9 AS SELECT $268435455, $268435456; +ERROR: there is no parameter $268435456 +LINE 1: PREPARE q9 AS SELECT $268435455, $268435456; + ^ -- test DEALLOCATE ALL; DEALLOCATE ALL; SELECT name, statement, parameter_types FROM pg_prepared_statements diff --git a/src/test/regress/sql/prepare.sql b/src/test/regress/sql/prepare.sql index c6098dc95c..1536f802d5 100644 --- a/src/test/regress/sql/prepare.sql +++ b/src/test/regress/sql/prepare.sql @@ -78,6 +78,9 @@ PREPARE q8 AS SELECT name, statement, parameter_types, result_types FROM pg_prepared_statements ORDER BY name; +-- max parameter number and one above +PREPARE q9 AS SELECT $268435455, $268435456; + -- test DEALLOCATE ALL; DEALLOCATE ALL; SELECT name, statement, parameter_types FROM pg_prepared_statements -- 2.45.1