Skip to content

Commit 786ecbd

Browse files
Zexin Lihackorum
authored andcommitted
Introduce pg_parse_lsn() to validate LSN command-line options
pg_waldump (--start/--end), pg_recvlogical (--startpos/--endpos), and pg_receivewal (--endpos) parsed user-supplied WAL locations with sscanf("%X/%08X"), which accepts several forms of input that the backend's pg_lsn type rejects, and in each case proceeds with a location the user did not specify: * A first component wider than 32 bits overflows its uint32 argument, which is undefined behavior per C99 7.19.6.2p10; glibc keeps the low-order 32 bits, so --startpos 123456789/0 runs with 23456789/0. A component wider than 64 bits additionally saturates to ULONG_MAX before the truncation. * sscanf() succeeds without consuming the whole string, so trailing characters are silently ignored: 0/123456789 is read as 0/12345678 and 1/2/3 as 1/2. * The %X conversion follows strtoul()'s rules, so leading whitespace, signs, and "0x" prefixes are accepted: --endpos -1/0 runs with FFFFFFFF/0. Add pg_parse_lsn() to src/common, following the backend's pg_lsn_in_safe(): one to eight hex digits, a slash, one to eight hex digits, and nothing else. Use it for the three options above. Inputs the server accepts as pg_lsn are accepted unchanged; everything else now fails with each tool's existing "invalid WAL location" or "could not parse start/end position" error, so the error texts are unchanged. The other frontend parsers of the same shape (in pg_basebackup, pg_rewind, pg_combinebackup, and parse_manifest.c) read server-generated strings rather than command-line input and are left alone, as is the backend's pg_lsn_in_safe() itself. Add regression tests for the previously-accepted forms. Bug: #19598 Reported-by: Michael Malis <malis@pgrust.com> Suggested-by: Fujii Masao <masao.fujii@gmail.com> Discussion: https://postgr.es/m/19598-aa67c8f4331611b4@postgresql.org
1 parent db0c984 commit 786ecbd

10 files changed

Lines changed: 128 additions & 17 deletions

File tree

src/bin/pg_basebackup/pg_receivewal.c

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
#include "access/xlog_internal.h"
3131
#include "common/file_perm.h"
3232
#include "common/logging.h"
33+
#include "common/pg_parse_lsn.h"
3334
#include "fe_utils/option_utils.h"
3435
#include "getopt_long.h"
3536
#include "libpq-fe.h"
@@ -651,8 +652,6 @@ main(int argc, char **argv)
651652
int c;
652653
int option_index;
653654
char *db_name;
654-
uint32 hi,
655-
lo;
656655
pg_compress_specification compression_spec;
657656
char *compression_detail = NULL;
658657
char *compression_algorithm_str = "none";
@@ -689,9 +688,8 @@ main(int argc, char **argv)
689688
basedir = pg_strdup(optarg);
690689
break;
691690
case 'E':
692-
if (sscanf(optarg, "%X/%08X", &hi, &lo) != 2)
691+
if (!pg_parse_lsn(optarg, &endpos))
693692
pg_fatal("could not parse end position \"%s\"", optarg);
694-
endpos = ((uint64) hi) << 32 | lo;
695693
break;
696694
case 'h':
697695
dbhost = pg_strdup(optarg);

src/bin/pg_basebackup/pg_recvlogical.c

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020

2121
#include "common/file_perm.h"
2222
#include "common/logging.h"
23+
#include "common/pg_parse_lsn.h"
2324
#include "fe_utils/option_utils.h"
2425
#include "getopt_long.h"
2526
#include "libpq-fe.h"
@@ -729,8 +730,6 @@ main(int argc, char **argv)
729730
};
730731
int c;
731732
int option_index;
732-
uint32 hi,
733-
lo;
734733
char *db_name;
735734

736735
pg_logging_init(argv[0]);
@@ -801,14 +800,12 @@ main(int argc, char **argv)
801800
break;
802801
/* replication options */
803802
case 'I':
804-
if (sscanf(optarg, "%X/%08X", &hi, &lo) != 2)
803+
if (!pg_parse_lsn(optarg, &startpos))
805804
pg_fatal("could not parse start position \"%s\"", optarg);
806-
startpos = ((uint64) hi) << 32 | lo;
807805
break;
808806
case 'E':
809-
if (sscanf(optarg, "%X/%08X", &hi, &lo) != 2)
807+
if (!pg_parse_lsn(optarg, &endpos))
810808
pg_fatal("could not parse end position \"%s\"", optarg);
811-
endpos = ((uint64) hi) << 32 | lo;
812809
break;
813810
case 'o':
814811
{

src/bin/pg_basebackup/t/020_pg_receivewal.pl

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,14 @@
2424
# Sanity checks for command line options.
2525
$primary->command_fails(['pg_receivewal'],
2626
'pg_receivewal needs target directory specified');
27+
$primary->command_fails_like(
28+
[ 'pg_receivewal', '--endpos' => '123456789/0' ],
29+
qr/error: could not parse end position/,
30+
'end position with first component wider than 32 bits');
31+
$primary->command_fails_like(
32+
[ 'pg_receivewal', '--endpos' => '1/2/3' ],
33+
qr/error: could not parse end position/,
34+
'end position with trailing garbage');
2735
$primary->command_fails(
2836
[
2937
'pg_receivewal',

src/bin/pg_basebackup/t/030_pg_recvlogical.pl

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,22 @@
4343
'--start',
4444
],
4545
'no destination file');
46+
$node->command_fails_like(
47+
[ 'pg_recvlogical', '--startpos' => '123456789/0' ],
48+
qr/error: could not parse start position/,
49+
'start position with first component wider than 32 bits');
50+
$node->command_fails_like(
51+
[ 'pg_recvlogical', '--startpos' => '0x1/0' ],
52+
qr/error: could not parse start position/,
53+
'start position with 0x prefix');
54+
$node->command_fails_like(
55+
[ 'pg_recvlogical', '--endpos' => '0/123456789' ],
56+
qr/error: could not parse end position/,
57+
'end position with second component wider than 32 bits');
58+
$node->command_fails_like(
59+
[ 'pg_recvlogical', '--endpos' => '1/2/3' ],
60+
qr/error: could not parse end position/,
61+
'end position with trailing garbage');
4662

4763
$node->command_ok(
4864
[

src/bin/pg_waldump/pg_waldump.c

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
#include "common/file_perm.h"
2828
#include "common/file_utils.h"
2929
#include "common/logging.h"
30+
#include "common/pg_parse_lsn.h"
3031
#include "common/relpath.h"
3132
#include "getopt_long.h"
3233
#include "pg_waldump.h"
@@ -928,8 +929,6 @@ usage(void)
928929
int
929930
main(int argc, char **argv)
930931
{
931-
uint32 xlogid;
932-
uint32 xrecoff;
933932
XLogReaderState *xlogreader_state;
934933
XLogDumpPrivate private;
935934
XLogDumpConfig config;
@@ -1047,13 +1046,12 @@ main(int argc, char **argv)
10471046
config.filter_by_extended = true;
10481047
break;
10491048
case 'e':
1050-
if (sscanf(optarg, "%X/%08X", &xlogid, &xrecoff) != 2)
1049+
if (!pg_parse_lsn(optarg, &private.endptr))
10511050
{
10521051
pg_log_error("invalid WAL location: \"%s\"",
10531052
optarg);
10541053
goto bad_argument;
10551054
}
1056-
private.endptr = (uint64) xlogid << 32 | xrecoff;
10571055
break;
10581056
case 'f':
10591057
config.follow = true;
@@ -1145,14 +1143,12 @@ main(int argc, char **argv)
11451143
config.filter_by_extended = true;
11461144
break;
11471145
case 's':
1148-
if (sscanf(optarg, "%X/%08X", &xlogid, &xrecoff) != 2)
1146+
if (!pg_parse_lsn(optarg, &private.startptr))
11491147
{
11501148
pg_log_error("invalid WAL location: \"%s\"",
11511149
optarg);
11521150
goto bad_argument;
11531151
}
1154-
else
1155-
private.startptr = (uint64) xlogid << 32 | xrecoff;
11561152
break;
11571153
case 't':
11581154

src/bin/pg_waldump/t/001_basic.pl

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,22 @@
5454
[ 'pg_waldump', '--end' => 'bad' ],
5555
qr/error: invalid WAL location/,
5656
'invalid end LSN');
57+
command_fails_like(
58+
[ 'pg_waldump', '--start' => '123456789/0' ],
59+
qr/error: invalid WAL location/,
60+
'start LSN with first component wider than 32 bits');
61+
command_fails_like(
62+
[ 'pg_waldump', '--start' => '0/123456789' ],
63+
qr/error: invalid WAL location/,
64+
'start LSN with second component wider than 32 bits');
65+
command_fails_like(
66+
[ 'pg_waldump', '--end' => '1/2/3' ],
67+
qr/error: invalid WAL location/,
68+
'end LSN with trailing garbage');
69+
command_fails_like(
70+
[ 'pg_waldump', '--end' => '0x1/0' ],
71+
qr/error: invalid WAL location/,
72+
'end LSN with 0x prefix');
5773

5874
# rmgr list: If you add one to the list, consider also adding a test
5975
# case exercising the new rmgr below.

src/common/Makefile

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@ OBJS_COMMON = \
7070
percentrepl.o \
7171
pg_get_line.o \
7272
pg_lzcompress.o \
73+
pg_parse_lsn.o \
7374
pg_prng.o \
7475
pgfnames.o \
7576
psprintf.o \

src/common/meson.build

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ common_sources = files(
2424
'percentrepl.c',
2525
'pg_get_line.c',
2626
'pg_lzcompress.c',
27+
'pg_parse_lsn.c',
2728
'pg_prng.c',
2829
'pgfnames.c',
2930
'psprintf.c',

src/common/pg_parse_lsn.c

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
/*-------------------------------------------------------------------------
2+
*
3+
* pg_parse_lsn.c
4+
* Parse a WAL location (LSN) in its text form.
5+
*
6+
* Portions Copyright (c) 1996-2026, PostgreSQL Global Development Group
7+
* Portions Copyright (c) 1994, Regents of the University of California
8+
*
9+
* IDENTIFICATION
10+
* src/common/pg_parse_lsn.c
11+
*
12+
*-------------------------------------------------------------------------
13+
*/
14+
15+
#ifndef FRONTEND
16+
#include "postgres.h"
17+
#else
18+
#include "postgres_fe.h"
19+
#endif
20+
21+
#include "common/pg_parse_lsn.h"
22+
23+
/* same limit as in the backend's pg_lsn.c */
24+
#define MAXPG_LSNCOMPONENT 8
25+
26+
/*
27+
* pg_parse_lsn
28+
*
29+
* Parse a WAL location in the "%X/%X" text form used for pg_lsn values,
30+
* requiring one to eight hexadecimal digits in each component and nothing
31+
* else, exactly as the backend's pg_lsn_in_safe() does. sscanf() is not
32+
* strict enough for this purpose: its %X conversion has no field-width
33+
* bound, so a component wider than 32 bits silently overflows a uint32
34+
* argument, and it also accepts leading whitespace, signs, and "0x"
35+
* prefixes, and does not insist on consuming the whole string.
36+
*
37+
* Returns true and sets *result on success; returns false on syntax
38+
* error, leaving *result unchanged.
39+
*/
40+
bool
41+
pg_parse_lsn(const char *str, XLogRecPtr *result)
42+
{
43+
int len1,
44+
len2;
45+
46+
len1 = strspn(str, "0123456789abcdefABCDEF");
47+
if (len1 < 1 || len1 > MAXPG_LSNCOMPONENT || str[len1] != '/')
48+
return false;
49+
50+
len2 = strspn(str + len1 + 1, "0123456789abcdefABCDEF");
51+
if (len2 < 1 || len2 > MAXPG_LSNCOMPONENT || str[len1 + 1 + len2] != '\0')
52+
return false;
53+
54+
*result = ((uint64) strtoul(str, NULL, 16)) << 32 |
55+
(uint32) strtoul(str + len1 + 1, NULL, 16);
56+
57+
return true;
58+
}

src/include/common/pg_parse_lsn.h

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
/*-------------------------------------------------------------------------
2+
*
3+
* pg_parse_lsn.h
4+
* Parse a WAL location (LSN) in its text form.
5+
*
6+
* Portions Copyright (c) 1996-2026, PostgreSQL Global Development Group
7+
* Portions Copyright (c) 1994, Regents of the University of California
8+
*
9+
* src/include/common/pg_parse_lsn.h
10+
*
11+
*-------------------------------------------------------------------------
12+
*/
13+
#ifndef PG_PARSE_LSN_H
14+
#define PG_PARSE_LSN_H
15+
16+
#include "access/xlogdefs.h"
17+
18+
extern bool pg_parse_lsn(const char *str, XLogRecPtr *result);
19+
20+
#endif /* PG_PARSE_LSN_H */

0 commit comments

Comments
 (0)