Backup manifests accept out-of-range LSNs
Hackorum builds and tests every patch posted to the lists, not only commitfest submissions. This is Hackorum's own CI rather than the PostgreSQL project's, and it is still under testing - please report anything that looks wrong.
You can run a PostgreSQL built from this patch straight from Docker, with no checkout and no build:
docker run --rm -p 5432:5432 ghcr.io/hackorum-dev/postgres-patch:t253315psql -h localhost -U postgresBuilt from patchset v3 (message #3), August 17, 2026 at 09:47 AM.
Hi,
355814931141 tightened the integer fields of a backup manifest, but
parse_xlogrecptr() in the same file still does:
if (sscanf(input, "%X/%08X", &hi, &lo) != 2)
"%X" stores into a uint32, so an oversized half is accepted with its
high bits dropped, and trailing text is ignored, since sscanf() still
returns 2. "123456789/0" is read as 23456789/0 and "1/2garbage" as
1/2, while "oops" is rejected as before.
That value validates the WAL ranges of a backup, in pg_verifybackup,
pg_combinebackup and the server's incremental backup path, so those
checks end up using a location the manifest doesn't contain.
The attached requires one to eight hex digits per half and the whole
string consumed, matching pg_lsn_in_safe(), and adds a few cases to
005_bad_manifest.pl. I stayed away from the strtoul()/endptr idiom the
neighbouring parsers use, since strtoul("-1", ..., 16) returns
ULONG_MAX without setting ERANGE, so "-1/0" would slip through as
FFFFFFFF/0.
Thoughts?
Regards,
Ayush
Attachments:
0001-Reject-out-of-range-LSNs-in-backup-manifests.patchapplication/octet-stream; name=0001-Reject-out-of-range-LSNs-in-backup-manifests.patchDownload+32-2
Reviewed this patch.
The issue seems valid. PostgreSQL-generated manifests should not normally
contain such LSNs, but a malformed or modified manifest could make the
parser operate on a different LSN than the one specified. The current
sscanf() based parser allows this through truncation of oversized
components and ignored trailing characters.
The proposed fix looks good and the tests cover the regression cases.
Show quoted text
Hi,
On Thu, 6 Aug 2026 at 07:22, Yuefei Shi <shiyuefei1004@gmail.com> wrote:
Reviewed this patch.
The issue seems valid. PostgreSQL-generated manifests should not normally
contain such LSNs, but a malformed or modified manifest could make the
parser operate on a different LSN than the one specified. The current
sscanf() based parser allows this through truncation of oversized
components and ignored trailing characters.The proposed fix looks good and the tests cover the regression cases.
Thanks for the review!
Attaching v2 which uses the new pg_parse_lsn funct.
Regards,
Ayush