1
0
mirror of https://github.com/postgres/postgres.git synced 2025-12-10 14:22:35 +03:00

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 undesirable, so tighten up the parsing to reject them.

Reviewed-by: Chao Li <li.evan.chao@gmail.com>
Discussion: https://www.postgresql.org/message-id/81adf5f3-36ad-4bcd-9ba5-1b95c7b7a807@iki.fi
This commit is contained in:
Heikki Linnakangas
2025-12-08 16:54:50 +02:00
parent 7f88553cea
commit 636c1914b4
2 changed files with 78 additions and 18 deletions

View File

@@ -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;
}

View File

@@ -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(
@@ -122,13 +122,20 @@ command_fails_like(
# This used to be forbidden, but nextMulti can legitimately be 0 after
# wraparound, so we now accept it in pg_resetwal too.
command_ok([ 'pg_resetwal', '-m' => '0,10', $node->data_dir ],
'succeeds with -m value 0 part 1');
command_ok(
[ 'pg_resetwal', '-m' => '0,10', $node->data_dir ],
'succeeds with -m value 0 in the first part');
# -0 doesn't make sense however
command_fails_like(
[ 'pg_resetwal', '-m' => '-0,10', $node->data_dir ],
qr/error: invalid argument for option -m/,
'fails with -m value -0 in the first part');
command_fails_like(
[ 'pg_resetwal', '-m' => '10,0', $node->data_dir ],
qr/must not be 0/,
'fails with -m value 0 part 2');
'fails with -m value 0 in the second part');
# -o
command_fails_like(
[ 'pg_resetwal', '-o' => 'foo', $node->data_dir ],
@@ -145,7 +152,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 +182,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 ],