BUG #19598: pg_waldump: -s/-e accept out-of-range WAL locations and silently use the low 32 bits

On Sun, Aug 2, 2026, Michael Malis wrote:

Note the echoed value: the 9-hex-digit input 123456789 was silently
reduced to 23456789.

Thanks for the report -- reproduced on current master (03f420c37f).
Patch attached.

The != 2 guard only fires when fewer than two conversions complete,
i.e. when %X or the '/' literal fails outright, as with "ZZZ/0". It
cannot reject inputs where both conversions succeed on the wrong
bytes, which happens through three properties of sscanf():

1. The first %X carries no field width, so a component wider than 32
bits overflows the uint32 argument -- undefined behavior per C99
7.19.6.2p10; glibc keeps the low-order 32 bits, which is the mangling
you observed. A component wider than 64 bits saturates to ULONG_MAX
at the strtoul() level first, so FFFFFFFFFFFFFFFFFFFF/0 runs as
FFFFFFFF/0.

2. sscanf() succeeds without consuming the whole string. On master
the low component is %08X, so "0/123456789" stops after eight digits
and runs as 0/12345678 ("1/2/3" runs as 1/2); on 18.x, with a bare %X
there, the same input instead wraps modulo 2^32.

3. %X follows strtoul()'s subject-sequence rules, accepting leading
whitespace, signs, and "0x" prefixes: "-1/0" runs as FFFFFFFF/0.

The patch replaces the two sscanf() calls with a static helper that
follows the backend's parser for this syntax, pg_lsn_in_safe() in
src/backend/utils/adt/pg_lsn.c: strspn() over the hex charset, one to
eight digits per component, separator and terminating NUL checked by
position. The patch intentionally does not change the "invalid WAL
location" error text, the treatment of any input the server considers
valid (including 8-digit and mixed-case components), or the
already-rejected cases, so scripts matching on the error text are
unaffected.

Measured on master against a real segment. After the patch, -s/-e
accept exactly what the server accepts as pg_lsn -- one to eight hex
digits, a slash, one to eight hex digits, nothing else. Each line
shows unpatched behavior first, patched behavior second:

123456789/0: ran as 23456789/00000000; now rejected
FFFFFFFFFFFFFFFFFFFF/0: ran as FFFFFFFF/00000000; now rejected
0/123456789: ran as 0/12345678; now rejected
1/2/3: ran as 1/00000002; now rejected
0x1/0: ran as 1/00000000; now rejected
-1/0: ran as FFFFFFFF/00000000; now rejected
" 1/0": ran as 1/00000000; now rejected

"Rejected" is the existing "invalid WAL location" error; all seven
inputs are already rejected by the server when cast to pg_lsn, so the
tool and the server now agree on every string. Unchanged: valid
inputs (0/1000028, 0/0, FFFFFFFF/FFFFFFFF, abcdef/ABCDEF) parse as
before, and inputs that already failed ("bad", "ZZZ/0") keep failing
the same way.

Regression tests are included next to the existing invalid-LSN checks;
without the fix the four new cases fail. The pg_waldump TAP suite and
make check pass here.

The same pattern parses user-supplied LSNs in pg_recvlogical (-I/-E)
and pg_receivewal (-E); pg_basebackup and pg_rewind only parse
server-returned strings. I kept this patch to pg_waldump to match
the report's scope, and can send a follow-up moving the helper next
to option_parse_int() in fe_utils to cover the other two if that
seems worthwhile.

Regards,
Zexin Li

On Mon, Aug 03, 2026 03:12 AM, PG Bug reporting form <noreply@postgresql.org>
wrote:

On Tue, Aug 4, 2026 at 5:54 PM Zexin Li <lizi.openmind@gmail.com> wrote:

The same pattern parses user-supplied LSNs in pg_recvlogical (-I/-E)
and pg_receivewal (-E); pg_basebackup and pg_rewind only parse
server-returned strings. I kept this patch to pg_waldump to match
the report's scope, and can send a follow-up moving the helper next
to option_parse_int() in fe_utils to cover the other two if that
seems worthwhile.

I think it would be better to improve pg_recvlogical and pg_receivewal as well,
not just pg_waldump, by introducing a common LSN parsing helper in,
for example, src/common. That would let frontend tools share exactly
the same LSN syntax checks.

pg_basebackup, pg_verifybackup, pg_rewind, and pg_combinebackup also parse LSN
but are a bit different, since they mostly parse LSNs from server responses,
backup manifests, backup_label files, or timeline history files rather than
direct command-line input. So they're less likely to see arbitrary invalid LSNs
from users.

Still, if we introduce a common LSN parser, it seems worth considering
converting those existing sscanf("%X/%08X") call sites as well. That would
make malformed metadata fail earlier and avoid having several slightly
different LSN parsers in frontend code. This should be done as a separate
patch from the pg_waldump/pg_recvlogical/pg_receivewal improvement,
though.

BTW, at least for me this looks more like an improvement than a bug fix.
So I think it should target v20devel.

Regards,

--
Fujii Masao

On Wed, Aug 5, 2026, Fujii Masao wrote:

I think it would be better to improve pg_recvlogical and pg_receivewal
as well, not just pg_waldump, by introducing a common LSN parsing
helper in, for example, src/common. That would let frontend tools
share exactly the same LSN syntax checks.

Agreed -- v2 attached, done that way.

The helper is pg_parse_lsn() in the new src/common/pg_parse_lsn.c,
with the same rules as the backend's pg_lsn_in_safe(): one to eight
hex digits, a slash, one to eight hex digits, and nothing else.
pg_waldump's static helper from v1 moves there, and pg_recvlogical
(-I/-E) and pg_receivewal (-E) now go through it as well.

Three choices worth calling out:

* The helper only parses and returns bool; each tool keeps its own
existing error message ("invalid WAL location" in pg_waldump,
"could not parse start/end position" in the other two), so no error
text changes anywhere. This follows the existing split between
strtoint() in src/common and option_parse_int() in fe_utils.

* No endptr-style variant yet. The three command-line options all
want whole-string parsing. Among the call sites for the separate
patch you describe, pg_rewind's timeline.c and pg_combinebackup's
backup_label.c parse an LSN as a prefix of a longer line, so that
patch will want a second entry point taking an endptr; I did not
add an API with no in-tree caller here. One more data point for
it: parse_manifest.c contains one more sscanf of the same shape,
and it lives in src/common itself, out of reach of fe_utils code
-- which also argues for src/common as the helper's home.

* The backend's pg_lsn_in_safe() is left untouched for now.

The behavior change is confined to the three options: strings the
server rejects as pg_lsn (overlong components, trailing garbage,
leading whitespace, signs, 0x prefixes) now fail with each tool's
existing error instead of running with a mangled location. Strings
the server accepts parse exactly as before; I re-ran the v1 input
matrix against all three tools to check both directions.

The new TAP cases for pg_recvlogical and pg_receivewal fail without
the code change and pass with it; the pg_waldump cases from v1 are
kept. make check-world, a full meson build, and git am on current
master are all clean here.

BTW, at least for me this looks more like an improvement than a bug fix.
So I think it should target v20devel.

Makes sense. I'll register the patch in the September commitfest.

I'd appreciate any feedback .

Regards,
Zexin Li

On Wed, Aug 05, 2026 03:03 PM, Fujii Masao <masao.fujii@gmail.com> wrote:

On Tue, Aug 4, 2026 at 5:54 PM Zexin Li <lizi.openmind@gmail.com> wrote:

The same pattern parses user-supplied LSNs in pg_recvlogical (-I/-E)
and pg_receivewal (-E); pg_basebackup and pg_rewind only parse
server-returned strings. I kept this patch to pg_waldump to match
the report's scope, and can send a follow-up moving the helper next
to option_parse_int() in fe_utils to cover the other two if that
seems worthwhile.

I think it would be better to improve pg_recvlogical and pg_receivewal as
well,
not just pg_waldump, by introducing a common LSN parsing helper in,
for example, src/common. That would let frontend tools share exactly
the same LSN syntax checks.

pg_basebackup, pg_verifybackup, pg_rewind, and pg_combinebackup also parse
LSN
but are a bit different, since they mostly parse LSNs from server
responses,
backup manifests, backup_label files, or timeline history files rather

than

direct command-line input. So they're less likely to see arbitrary invalid
LSNs
from users.

Still, if we introduce a common LSN parser, it seems worth considering
converting those existing sscanf("%X/%08X") call sites as well. That would
make malformed metadata fail earlier and avoid having several slightly
different LSN parsers in frontend code. This should be done as a separate
patch from the pg_waldump/pg_recvlogical/pg_receivewal improvement,
though.

BTW, at least for me this looks more like an improvement than a bug fix.
So I think it should target v20devel.

Regards,

--
Fujii Masao

On Wed, Aug 05, 2026 03:03 PM, Fujii Masao <masao.fujii@gmail.com> wrote:

On Tue, Aug 4, 2026 at 5:54 PM Zexin Li <lizi.openmind@gmail.com> wrote:

The same pattern parses user-supplied LSNs in pg_recvlogical (-I/-E)
and pg_receivewal (-E); pg_basebackup and pg_rewind only parse
server-returned strings. I kept this patch to pg_waldump to match
the report's scope, and can send a follow-up moving the helper next
to option_parse_int() in fe_utils to cover the other two if that
seems worthwhile.

I think it would be better to improve pg_recvlogical and pg_receivewal as
well,
not just pg_waldump, by introducing a common LSN parsing helper in,
for example, src/common. That would let frontend tools share exactly
the same LSN syntax checks.

pg_basebackup, pg_verifybackup, pg_rewind, and pg_combinebackup also parse
LSN
but are a bit different, since they mostly parse LSNs from server
responses,
backup manifests, backup_label files, or timeline history files rather than
direct command-line input. So they're less likely to see arbitrary invalid
LSNs
from users.

Still, if we introduce a common LSN parser, it seems worth considering
converting those existing sscanf("%X/%08X") call sites as well. That would
make malformed metadata fail earlier and avoid having several slightly
different LSN parsers in frontend code. This should be done as a separate
patch from the pg_waldump/pg_recvlogical/pg_receivewal improvement,
though.

BTW, at least for me this looks more like an improvement than a bug fix.
So I think it should target v20devel.

Regards,

--
Fujii Masao