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
