From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 From: Yuriy Grigoryev Date: Tue, 29 Sep 2026 00:00:00 +0700 Subject: [PATCH v2 1/2] Reject WAL block images whose hole does not fit DecodeXLogRecord() cross-checked a block image with BKPIMAGE_HAS_HOLE only for non-zero hole_offset/hole_length and bimg_len < BLCKSZ, but never checked that the hole itself fits in the page. Both fields are uint16s taken from the record, so e.g. hole_offset = 8000, hole_length = 8000 (sum 16000 > BLCKSZ) with a tiny bimg_len passed validation. RestoreBlockImage() then used them as memcpy/MemSet lengths, with the last length (BLCKSZ - (hole_offset + hole_length)) underflowing to a huge size_t, and the decompressors were told the output buffer was BLCKSZ - hole_length bytes. This covers both shapes: compressed images, where hole_length is stored in the WAL, and uncompressed images, where hole_length is derived as BLCKSZ - bimg_len and nothing checked hole_offset against bimg_len. For the uncompressed case the new bound amounts to requiring hole_offset <= bimg_len. Add the missing bound to the existing HAS_HOLE check: hole_offset > BLCKSZ || hole_length > BLCKSZ - hole_offset using subtraction so the two untrusted uint16s are never added together. A bad hole keeps the existing HAS_HOLE error message. DecodeXLogRecord() is the only place that fills in the image fields of DecodedBkpBlock, so re-check the same condition with Asserts at the start of RestoreBlockImage(), before decompression or memcpy. XLogRecordAssemble() only creates a hole when pd_lower >= SizeOfPageHeaderData, pd_upper > pd_lower and pd_upper <= BLCKSZ, so no WAL written by PostgreSQL can fail the new check. Reported-by: Michael Malis Reviewed-by: Rahul Yadav Discussion: https://postgr.es/m/cc0a5886d2a74f99ae58c1647ea5ed8b@localhost.localdomain --- src/backend/access/transam/xlogreader.c | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/src/backend/access/transam/xlogreader.c b/src/backend/access/transam/xlogreader.c --- a/src/backend/access/transam/xlogreader.c +++ b/src/backend/access/transam/xlogreader.c @@ -1874,13 +1874,18 @@ datatotal += blk->bimg_len; /* - * cross-check that hole_offset > 0, hole_length > 0 and - * bimg_len < BLCKSZ if the HAS_HOLE flag is set. + * cross-check that hole_offset > 0, hole_length > 0, + * bimg_len < BLCKSZ, and the hole fits in the page if the + * HAS_HOLE flag is set. Compare hole_length with + * BLCKSZ - hole_offset so the two untrusted fields are never + * added together. */ if ((blk->bimg_info & BKPIMAGE_HAS_HOLE) && (blk->hole_offset == 0 || blk->hole_length == 0 || - blk->bimg_len == BLCKSZ)) + blk->bimg_len == BLCKSZ || + blk->hole_offset > BLCKSZ || + blk->hole_length > BLCKSZ - blk->hole_offset)) { report_invalid_record(state, "BKPIMAGE_HAS_HOLE set, but hole offset %d length %d block image length %d at %X/%08X", @@ -2149,6 +2154,15 @@ bkpb = &record->record->blocks[block_id]; ptr = bkpb->bkp_image; + /* + * The hole must fit in the page. DecodeXLogRecord() already enforces + * this for all records that reach here, so this cannot fail. Assert + * it before using the values as memcpy/MemSet lengths or as the + * decompressor output capacity. + */ + Assert(bkpb->hole_offset <= BLCKSZ); + Assert(bkpb->hole_length <= BLCKSZ - bkpb->hole_offset); + if (BKPIMAGE_COMPRESSED(bkpb->bimg_info)) { /* If a backup block image is compressed, decompress it */ -- 2.43.0