On 16.05.24 01:11, Michael Paquier wrote:
On Wed, May 15, 2024 at 01:59:36PM +0200, Peter Eisentraut wrote:
On 14.05.24 18:07, Erik Wienhold wrote:
Patch 0002 replaces atol with pg_strtoint32_safe in the backend parser
and strtoint in ECPG. This fixes overflows like:
Seems like a good idea, but as was said, this is an older issue, so let's
look at that separately.
Hmm, yeah. I would be really tempted to fix that now.
Now, it has been this way for ages, and with my RMT hat on (aka I need
to show the example), I'd suggest to wait for when the v18 branch
opens as there is no urgency. I'm OK to apply it myself at the end,
the patch is a good idea.
On this specific patch, maybe reword "parameter too large" to "parameter
number too large".
Also, I was bemused by the use of atol(), which is notoriously
unportable (sizeof(long)). So I poked around and found more places that
might need fixing. I'm attaching a patch here with annotations too look
at later.
From 2d3ad223b1f9b7bb5bebc6b6ef8cbba0d1c0022b Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pe...@eisentraut.org>
Date: Thu, 16 May 2024 08:36:21 +0200
Subject: [PATCH] WIP: atol() investigation
---
src/backend/parser/scan.l | 2 +-
src/bin/pg_basebackup/pg_basebackup.c | 2 +-
src/bin/pg_basebackup/streamutil.c | 2 +-
src/bin/pg_rewind/libpq_source.c | 2 +-
src/interfaces/ecpg/ecpglib/execute.c | 2 +-
src/interfaces/ecpg/preproc/ecpg.trailer | 2 +-
src/interfaces/ecpg/preproc/pgc.l | 2 +-
7 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/src/backend/parser/scan.l b/src/backend/parser/scan.l
index b499975e9c4..383d3f2d39a 100644
--- a/src/backend/parser/scan.l
+++ b/src/backend/parser/scan.l
@@ -993,7 +993,7 @@ other .
{param} {
SET_YYLLOC();
- yylval->ival = atol(yytext + 1);
+ yylval->ival = atol(yytext + 1); //
FIXME with overflow check
return PARAM;
}
{param_junk} {
diff --git a/src/bin/pg_basebackup/pg_basebackup.c
b/src/bin/pg_basebackup/pg_basebackup.c
index 8f3dd04fd22..4ebc5e3b2b8 100644
--- a/src/bin/pg_basebackup/pg_basebackup.c
+++ b/src/bin/pg_basebackup/pg_basebackup.c
@@ -2056,7 +2056,7 @@ BaseBackup(char *compression_algorithm, char
*compression_detail,
tablespacecount = PQntuples(res);
for (i = 0; i < PQntuples(res); i++)
{
- totalsize_kb += atol(PQgetvalue(res, i, 2));
+ totalsize_kb += atoll(PQgetvalue(res, i, 2)); // FIXME: atol()
might truncate if sizeof(long)==4
/*
* Verify tablespace directories are empty. Don't bother with
the
diff --git a/src/bin/pg_basebackup/streamutil.c
b/src/bin/pg_basebackup/streamutil.c
index d0efd8600ca..1d303d16ef5 100644
--- a/src/bin/pg_basebackup/streamutil.c
+++ b/src/bin/pg_basebackup/streamutil.c
@@ -631,7 +631,7 @@ GetSlotInformation(PGconn *conn, const char *slot_name,
/* current TLI */
if (!PQgetisnull(res, 0, 2))
- tli_loc = (TimeLineID) atol(PQgetvalue(res, 0, 2));
+ tli_loc = (TimeLineID) atol(PQgetvalue(res, 0, 2)); // FIXME:
use strtoul()
PQclear(res);
diff --git a/src/bin/pg_rewind/libpq_source.c b/src/bin/pg_rewind/libpq_source.c
index 7d898c3b501..618b175dcc4 100644
--- a/src/bin/pg_rewind/libpq_source.c
+++ b/src/bin/pg_rewind/libpq_source.c
@@ -294,7 +294,7 @@ libpq_traverse_files(rewind_source *source,
process_file_callback_t callback)
}
path = PQgetvalue(res, i, 0);
- filesize = atol(PQgetvalue(res, i, 1));
+ filesize = atoll(PQgetvalue(res, i, 1)); // FIXME: atol() might
truncate
isdir = (strcmp(PQgetvalue(res, i, 2), "t") == 0);
link_target = PQgetvalue(res, i, 3);
diff --git a/src/interfaces/ecpg/ecpglib/execute.c
b/src/interfaces/ecpg/ecpglib/execute.c
index 04d0b40c537..c578c21cf66 100644
--- a/src/interfaces/ecpg/ecpglib/execute.c
+++ b/src/interfaces/ecpg/ecpglib/execute.c
@@ -278,7 +278,7 @@ ecpg_is_type_an_array(int type, const struct statement
*stmt, const struct varia
isarray = ECPG_ARRAY_NONE;
else
{
- isarray = (atol((char *) PQgetvalue(query, 0, 0)) ==
-1) ? ECPG_ARRAY_ARRAY : ECPG_ARRAY_VECTOR;
+ isarray = (atoi((char *) PQgetvalue(query, 0, 0)) ==
-1) ? ECPG_ARRAY_ARRAY : ECPG_ARRAY_VECTOR;
if (ecpg_dynamic_type(type) == SQL3_CHARACTER ||
ecpg_dynamic_type(type) ==
SQL3_CHARACTER_VARYING)
{
diff --git a/src/interfaces/ecpg/preproc/ecpg.trailer
b/src/interfaces/ecpg/preproc/ecpg.trailer
index b2aa44f36dd..8ac1c5c9eda 100644
--- a/src/interfaces/ecpg/preproc/ecpg.trailer
+++ b/src/interfaces/ecpg/preproc/ecpg.trailer
@@ -217,7 +217,7 @@ char_variable: cvariable
enum ECPGttype type = p->type->type;
/* If we have just one character this is not a string */
- if (atol(p->type->size) == 1)
+ if (atoi(p->type->size) == 1)
mmerror(PARSE_ERROR, ET_ERROR, "invalid
data type");
else
{
diff --git a/src/interfaces/ecpg/preproc/pgc.l
b/src/interfaces/ecpg/preproc/pgc.l
index d117cafce65..8f252741433 100644
--- a/src/interfaces/ecpg/preproc/pgc.l
+++ b/src/interfaces/ecpg/preproc/pgc.l
@@ -938,7 +938,7 @@ cppline
{space}*#([^i][A-Za-z]*|{if}|{ifdef}|{ifndef}|{import})((\/\*[^*/]*\*+
}
{param} {
- base_yylval.ival = atol(yytext+1);
+ base_yylval.ival = atol(yytext+1); //
FIXME with overflow check
return PARAM;
}
{param_junk} {
--
2.44.0
