From c0c07bcf36c7d9c06ca45619ccca5ca38010e483 Mon Sep 17 00:00:00 2001 From: Heikki Linnakangas Date: Wed, 19 Nov 2025 16:36:00 +0200 Subject: [PATCH 1/2] pg_resetwal: Reject negative and out of range arguments The strtoul() function that we used to parse many of the options accepts negative values, and silently wraps them to the equivalent unsigned values. For example, -1 becomes 0xFFFFFFFF, on platforms where unsigned long is 32 bits wide. Also, on platforms where "unsigned long" is 64 bits wide, we silently casted values larger than UINT32_MAX to the equivalent 32-bit value. Both of those behaviors seem undesireable, so tighten up the parsing to reject negative and too large values. --- src/bin/pg_resetwal/pg_resetwal.c | 64 ++++++++++++++++++++++++------ src/bin/pg_resetwal/t/001_basic.pl | 19 ++++++++- 2 files changed, 68 insertions(+), 15 deletions(-) diff --git a/src/bin/pg_resetwal/pg_resetwal.c b/src/bin/pg_resetwal/pg_resetwal.c index 8d5d9805279..8ca8dad01a0 100644 --- a/src/bin/pg_resetwal/pg_resetwal.c +++ b/src/bin/pg_resetwal/pg_resetwal.c @@ -92,6 +92,7 @@ static void KillExistingArchiveStatus(void); static void KillExistingWALSummaries(void); static void WriteEmptyXLOG(void); static void usage(void); +static uint32 strtouint32_strict(const char *restrict s, char **restrict endptr, int base); int @@ -120,7 +121,6 @@ main(int argc, char *argv[]) MultiXactId set_oldestmxid = 0; char *endptr; char *endptr2; - int64 tmpi64; char *DataDir = NULL; char *log_fname = NULL; int fd; @@ -162,7 +162,7 @@ main(int argc, char *argv[]) case 'e': errno = 0; - set_xid_epoch = strtoul(optarg, &endptr, 0); + set_xid_epoch = strtouint32_strict(optarg, &endptr, 0); if (endptr == optarg || *endptr != '\0' || errno != 0) { /*------ @@ -177,7 +177,7 @@ main(int argc, char *argv[]) case 'u': errno = 0; - set_oldest_xid = strtoul(optarg, &endptr, 0); + set_oldest_xid = strtouint32_strict(optarg, &endptr, 0); if (endptr == optarg || *endptr != '\0' || errno != 0) { pg_log_error("invalid argument for option %s", "-u"); @@ -190,7 +190,7 @@ main(int argc, char *argv[]) case 'x': errno = 0; - set_xid = strtoul(optarg, &endptr, 0); + set_xid = strtouint32_strict(optarg, &endptr, 0); if (endptr == optarg || *endptr != '\0' || errno != 0) { pg_log_error("invalid argument for option %s", "-x"); @@ -203,7 +203,7 @@ main(int argc, char *argv[]) case 'c': errno = 0; - set_oldest_commit_ts_xid = strtoul(optarg, &endptr, 0); + set_oldest_commit_ts_xid = strtouint32_strict(optarg, &endptr, 0); if (endptr == optarg || *endptr != ',' || errno != 0) { pg_log_error("invalid argument for option %s", "-c"); @@ -229,7 +229,7 @@ main(int argc, char *argv[]) case 'o': errno = 0; - set_oid = strtoul(optarg, &endptr, 0); + set_oid = strtouint32_strict(optarg, &endptr, 0); if (endptr == optarg || *endptr != '\0' || errno != 0) { pg_log_error("invalid argument for option %s", "-o"); @@ -242,7 +242,7 @@ main(int argc, char *argv[]) case 'm': errno = 0; - set_mxid = strtoul(optarg, &endptr, 0); + set_mxid = strtouint32_strict(optarg, &endptr, 0); if (endptr == optarg || *endptr != ',' || errno != 0) { pg_log_error("invalid argument for option %s", "-m"); @@ -250,7 +250,7 @@ main(int argc, char *argv[]) exit(1); } - set_oldestmxid = strtoul(endptr + 1, &endptr2, 0); + set_oldestmxid = strtouint32_strict(endptr + 1, &endptr2, 0); if (endptr2 == endptr + 1 || *endptr2 != '\0' || errno != 0) { pg_log_error("invalid argument for option %s", "-m"); @@ -269,17 +269,13 @@ main(int argc, char *argv[]) case 'O': errno = 0; - tmpi64 = strtoi64(optarg, &endptr, 0); + set_mxoff = strtouint32_strict(optarg, &endptr, 0); if (endptr == optarg || *endptr != '\0' || errno != 0) { pg_log_error("invalid argument for option %s", "-O"); pg_log_error_hint("Try \"%s --help\" for more information.", progname); exit(1); } - if (tmpi64 < 0 || tmpi64 > (int64) MaxMultiXactOffset) - pg_fatal("multitransaction offset (-O) must be between 0 and %u", MaxMultiXactOffset); - - set_mxoff = (MultiXactOffset) tmpi64; mxoff_given = true; break; @@ -1214,3 +1210,45 @@ usage(void) printf(_("\nReport bugs to <%s>.\n"), PACKAGE_BUGREPORT); printf(_("%s home page: <%s>\n"), PACKAGE_NAME, PACKAGE_URL); } + +/* + * strtouint32_strict -- like strtoul(), but returns uint32 and doesn't accept + * negative values + */ +static uint32 +strtouint32_strict(const char *restrict s, char **restrict endptr, int base) +{ + unsigned long val; + bool is_neg; + + /* skip leading whitespace */ + while (isspace(*s)) + s++; + + /* + * Is it negative? We still call strtoul() if it was, to set 'endptr'. + * (The current callers don't care though.) + */ + is_neg = (*s == '-'); + + val = strtoul(s, endptr, base); + + /* reject if it was negative */ + if (errno == 0 && is_neg) + { + errno = ERANGE; + val = 0; + } + + /* + * reject values larger than UINT32_MAX on platforms where long is 64 bits + * wide. + */ + if (errno == 0 && val != (uint32) val) + { + errno = ERANGE; + val = UINT32_MAX; + } + + return (uint32) val; +} diff --git a/src/bin/pg_resetwal/t/001_basic.pl b/src/bin/pg_resetwal/t/001_basic.pl index 90ecb8afe18..e9780dbe2a6 100644 --- a/src/bin/pg_resetwal/t/001_basic.pl +++ b/src/bin/pg_resetwal/t/001_basic.pl @@ -103,7 +103,7 @@ command_fails_like( 'fails with incorrect -e option'); command_fails_like( [ 'pg_resetwal', '-e' => '-1', $node->data_dir ], - qr/must not be -1/, + qr/error: invalid argument for option -e/, 'fails with -e value -1'); # -l command_fails_like( @@ -145,7 +145,7 @@ command_fails_like( 'fails with incorrect -O option'); command_fails_like( [ 'pg_resetwal', '-O' => '-1', $node->data_dir ], - qr/must be between 0 and 4294967295/, + qr/error: invalid argument for option -O/, 'fails with -O value -1'); # --wal-segsize command_fails_like( @@ -175,6 +175,21 @@ command_fails_like( qr/must be greater than/, 'fails with -x value too small'); +# Check out of range values with -x. These are forbidden for all other +# 32-bit values too, but we use just -x to exercise the parsing. +command_fails_like( + [ 'pg_resetwal', '-x' => '-1', $node->data_dir ], + qr/error: invalid argument for option -x/, + 'fails with -x value -1'); +command_fails_like( + [ 'pg_resetwal', '-x' => '-100', $node->data_dir ], + qr/error: invalid argument for option -x/, + 'fails with negative -x value'); +command_fails_like( + [ 'pg_resetwal', '-x' => '10000000000', $node->data_dir ], + qr/error: invalid argument for option -x/, + 'fails with -x value too large'); + # --char-signedness command_fails_like( [ 'pg_resetwal', '--char-signedness', 'foo', $node->data_dir ], -- 2.47.3