From: Grigorev Jurij <ju.grigorev@ftdata.ru>
To: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
CC: "pgsql-bugs@lists.postgresql.org" <pgsql-bugs@lists.postgresql.org>,
	Michael Malis <malis@pgrust.com>
Subject: Re: BUG #19599: RestoreBlockImage: the decode cross-checks never
 bound hole_offset + hole_length against BLCKSZ
Thread-Topic: BUG #19599: RestoreBlockImage: the decode cross-checks never
 bound hole_offset + hole_length against BLCKSZ
Thread-Index: AQHdPEAamNcHbRVPaUWRhlpYNzs5kA==
Date: Fri, 4 Sep 2026 08:01:05 +0000
Message-ID: <cc0a5886d2a74f99ae58c1647ea5ed8b@localhost.localdomain>
Accept-Language: ru-RU, en-US
Content-Language: ru-RU
Content-Type: multipart/mixed;
	boundary="_002_cc0a5886d2a74f99ae58c1647ea5ed8bftdataru_"
MIME-Version: 1.0
Archived-At: 
 <https://www.postgresql.org/message-id/cc0a5886d2a74f99ae58c1647ea5ed8b%40localhost.localdomain>
Precedence: bulk

--_002_cc0a5886d2a74f99ae58c1647ea5ed8bftdataru_
Content-Type: text/plain; charset="koi8-r"
Content-Transfer-Encoding: quoted-printable

Hi,

Michael reported this as BUG #19599.  He wasn't sure it was a real issue, b=
ut
I think the missing check is still worth fixing.  Postgres never writes a
hole that doesn't fit in the page, but DecodeXLogRecord() will happily
accept one from a corrupt or hand-built record, and RestoreBlockImage()
then uses those fields as memcpy/MemSet lengths.

What decode checks today is only non-zero values:

>     if ((blk->bimg_info & BKPIMAGE_HAS_HOLE) &&
>         (blk->hole_offset =3D=3D 0 ||
>          blk->hole_length =3D=3D 0 ||
>          blk->bimg_len =3D=3D BLCKSZ))

It never asks whether the hole actually fits in the page.  Both fields
are uint16s taken from the record, so this gets through:

    hole_offset =3D 8000
    hole_length =3D 8000   /* 16000 > BLCKSZ */
    bimg_len    =3D 16
    bimg_info   =3D HAS_HOLE | APPLY | COMPRESS_PGLZ

That's specifically a compressed image, because that's the only shape
where hole_length is stored in the WAL rather than computed.  The CRC
can still be valid.  After that, RestoreBlockImage() does:

    memcpy(page, ptr, hole_offset);
    MemSet(page + hole_offset, 0, hole_length);
    memcpy(..., BLCKSZ - (hole_offset + hole_length));

With the numbers above, the first memcpy already runs off the end of an
8kB page, and the last length underflows to a huge size_t.  The
decompressors have the same problem: they are told the output buffer is
BLCKSZ - hole_length bytes.

I've attached the patch that adds the missing bound to that existing HAS_HO=
LE
check:

    hole_offset > BLCKSZ ||
    hole_length > BLCKSZ - hole_offset

(subtraction rather than addition, so the two uint16s can't overflow.)
The same check is repeated at the start of RestoreBlockImage(), before
decompression or memcpy.  A bad hole still uses the existing HAS_HOLE
error message, I didn't add a new one.

I also have a small frontend test that builds this record in memory (valid
header and CRC, compressed image, bad hole) and feeds it to
DecodeXLogRecord().  Happy to send that if it's useful, I left it out of
this mail so the patch stays small.

Does this look like the right approach?
I'd also like to hear whether it's worth back-patching.

Thanks,
  Yuriy Grigoryev=

--_002_cc0a5886d2a74f99ae58c1647ea5ed8bftdataru_
Content-Type: application/octet-stream;
	name="0001-Reject-WAL-block-images-whose-hole-does-not-fit.patch"
Content-Description: 
 0001-Reject-WAL-block-images-whose-hole-does-not-fit.patch
Content-Disposition: attachment;
	filename="0001-Reject-WAL-block-images-whose-hole-does-not-fit.patch";
	size=3378; creation-date="Fri, 04 Sep 2026 07:49:32 GMT";
	modification-date="Fri, 04 Sep 2026 07:49:32 GMT"
Content-Transfer-Encoding: base64

RnJvbSAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwIE1vbiBTZXAgMTcg
MDA6MDA6MDAgMjAwMQpGcm9tOiBZdXJpeSBHcmlnb3J5ZXYgPGp1LmdyaWdvcmV2QGZ0ZGF0YS5y
dT4KRGF0ZTogRnJpLCA0IFNlcCAyMDI2IDE0OjIwOjAwICswNzAwClN1YmplY3Q6IFtQQVRDSF0g
UmVqZWN0IFdBTCBibG9jayBpbWFnZXMgd2hvc2UgaG9sZSBkb2VzIG5vdCBmaXQgaW4gdGhlIHBh
Z2UuCgpEZWNvZGVYTG9nUmVjb3JkKCkgYWxyZWFkeSByZWplY3RzIGEgQktQSU1BR0VfSEFTX0hP
TEUgaW1hZ2Ugd2hlbgpob2xlX29mZnNldCBvciBob2xlX2xlbmd0aCBpcyB6ZXJvLCBvciB3aGVu
IGJpbWdfbGVuIGlzIEJMQ0tTWi4gIEl0CmRvZXMgbm90IGNoZWNrIHRoYXQgdGhlIGhvbGUgbGll
cyBpbnNpZGUgdGhlIHBhZ2UuICBob2xlX29mZnNldCBhbmQKaG9sZV9sZW5ndGggYXJlIHVpbnQx
NiB2YWx1ZXMgdGFrZW4gZnJvbSB0aGUgV0FMIHJlY29yZCwgc28gYQpjb21wcmVzc2VkIGltYWdl
IGNhbiBjbGFpbSBhIGhvbGUgdGhhdCBzdGFydHMgbmVhciB0aGUgZW5kIG9mIHRoZQpwYWdlIGFu
ZCBleHRlbmRzIHdlbGwgcGFzdCBCTENLU1ouCgpSZXN0b3JlQmxvY2tJbWFnZSgpIHRoZW4gdXNl
cyB0aG9zZSBmaWVsZHMgYXMgbWVtY3B5L01lbVNldCBsZW5ndGhzCmludG8gYSBCTENLU1ogYnVm
ZmVyLCBhbmQgYXMgQkxDS1NaIC0gaG9sZV9sZW5ndGggZm9yIHRoZQpkZWNvbXByZXNzb3IncyBv
dXRwdXQgY2FwYWNpdHkuICBBIGhvbGUgdGhhdCBkb2VzIG5vdCBmaXQgY2F1c2VzCm91dC1vZi1i
b3VuZHMgd3JpdGVzIGFuZCBzaXplX3QgdW5kZXJmbG93LgoKRXh0ZW5kIHRoZSBleGlzdGluZyBI
QVNfSE9MRSBjcm9zcy1jaGVjayB3aXRoIGhvbGVfb2Zmc2V0IDw9IEJMQ0tTWgphbmQgaG9sZV9s
ZW5ndGggPD0gQkxDS1NaIC0gaG9sZV9vZmZzZXQsIHVzaW5nIHN1YnRyYWN0aW9uIHNvIHRoZQp0
d28gdW50cnVzdGVkIGZpZWxkcyBhcmUgbmV2ZXIgYWRkZWQgdG9nZXRoZXIuICBSZS1jaGVjayB0
aGUgc2FtZQpib3VuZCBhdCB0aGUgc3RhcnQgb2YgUmVzdG9yZUJsb2NrSW1hZ2UoKSBiZWZvcmUg
ZGVjb21wcmVzc2lvbiBvcgptZW1jcHkuCgpUaGlzIGlzIG5vdCByZWFjaGFibGUgZnJvbSBXQUwg
cHJvZHVjZWQgYnkgYSBoZWFsdGh5IFBvc3RncmVTUUwKaW5zdGFuY2UuICBJdCByZXF1aXJlcyBh
IGNvcnJ1cHQgb3IgY3JhZnRlZCByZWNvcmQgdGhhdCBzdGlsbCBoYXMgYQpwbGF1c2libGUgc3Ry
dWN0dXJlLgoKRGlzY3Vzc2lvbjogaHR0cHM6Ly9wb3N0Z3IuZXMvbS8xOTU5OS04ODU5YzM4MjJh
ODMxMzMxQHBvc3RncmVzcWwub3JnCi0tLQogc3JjL2JhY2tlbmQvYWNjZXNzL3RyYW5zYW0veGxv
Z3JlYWRlci5jIHwgMjQgKysrKysrKysrKysrKysrKysrKystLS0tCiAxIGZpbGUgY2hhbmdlZCwg
MjAgaW5zZXJ0aW9ucygrKSwgNCBkZWxldGlvbnMoLSkKCmRpZmYgLS1naXQgYS9zcmMvYmFja2Vu
ZC9hY2Nlc3MvdHJhbnNhbS94bG9ncmVhZGVyLmMgYi9zcmMvYmFja2VuZC9hY2Nlc3MvdHJhbnNh
bS94bG9ncmVhZGVyLmMKaW5kZXggNWMyNmQzM2E2MDMuLmI3ZDA4MmQ4ZjhiIDEwMDY0NAotLS0g
YS9zcmMvYmFja2VuZC9hY2Nlc3MvdHJhbnNhbS94bG9ncmVhZGVyLmMKKysrIGIvc3JjL2JhY2tl
bmQvYWNjZXNzL3RyYW5zYW0veGxvZ3JlYWRlci5jCkBAIC0xODI0LDEzICsxODI0LDE4IEBAIERl
Y29kZVhMb2dSZWNvcmQoWExvZ1JlYWRlclN0YXRlICpzdGF0ZSwKIAkJCQlkYXRhdG90YWwgKz0g
YmxrLT5iaW1nX2xlbjsKIAogCQkJCS8qCi0JCQkJICogY3Jvc3MtY2hlY2sgdGhhdCBob2xlX29m
ZnNldCA+IDAsIGhvbGVfbGVuZ3RoID4gMCBhbmQKLQkJCQkgKiBiaW1nX2xlbiA8IEJMQ0tTWiBp
ZiB0aGUgSEFTX0hPTEUgZmxhZyBpcyBzZXQuCisJCQkJICogY3Jvc3MtY2hlY2sgdGhhdCBob2xl
X29mZnNldCA+IDAsIGhvbGVfbGVuZ3RoID4gMCwKKwkJCQkgKiBiaW1nX2xlbiA8IEJMQ0tTWiwg
YW5kIHRoZSBob2xlIGZpdHMgaW4gdGhlIHBhZ2UgaWYgdGhlCisJCQkJICogSEFTX0hPTEUgZmxh
ZyBpcyBzZXQuICBDb21wYXJlIGhvbGVfbGVuZ3RoIHdpdGgKKwkJCQkgKiBCTENLU1ogLSBob2xl
X29mZnNldCBzbyB0aGUgdHdvIHVudHJ1c3RlZCBmaWVsZHMgYXJlIG5ldmVyCisJCQkJICogYWRk
ZWQgdG9nZXRoZXIuCiAJCQkJICovCiAJCQkJaWYgKChibGstPmJpbWdfaW5mbyAmIEJLUElNQUdF
X0hBU19IT0xFKSAmJgogCQkJCQkoYmxrLT5ob2xlX29mZnNldCA9PSAwIHx8CiAJCQkJCSBibGst
PmhvbGVfbGVuZ3RoID09IDAgfHwKLQkJCQkJIGJsay0+YmltZ19sZW4gPT0gQkxDS1NaKSkKKwkJ
CQkJIGJsay0+YmltZ19sZW4gPT0gQkxDS1NaIHx8CisJCQkJCSBibGstPmhvbGVfb2Zmc2V0ID4g
QkxDS1NaIHx8CisJCQkJCSBibGstPmhvbGVfbGVuZ3RoID4gQkxDS1NaIC0gYmxrLT5ob2xlX29m
ZnNldCkpCiAJCQkJewogCQkJCQlyZXBvcnRfaW52YWxpZF9yZWNvcmQoc3RhdGUsCiAJCQkJCQkJ
CQkJICAiQktQSU1BR0VfSEFTX0hPTEUgc2V0LCBidXQgaG9sZSBvZmZzZXQgJXUgbGVuZ3RoICV1
IGJsb2NrIGltYWdlIGxlbmd0aCAldSBhdCAlWC8lWCIsCkBAIC0yMDk5LDYgKzIxMDQsMjEgQEAg
UmVzdG9yZUJsb2NrSW1hZ2UoWExvZ1JlYWRlclN0YXRlICpyZWNvcmQsIHVpbnQ4IGJsb2NrX2lk
LCBjaGFyICpwYWdlKQogCWJrcGIgPSAmcmVjb3JkLT5yZWNvcmQtPmJsb2Nrc1tibG9ja19pZF07
CiAJcHRyID0gYmtwYi0+YmtwX2ltYWdlOwogCisJLyoKKwkgKiBUaGUgaG9sZSBtdXN0IGZpdCBp
biB0aGUgcGFnZS4gIERlY29kZVhMb2dSZWNvcmQoKSBhbHJlYWR5IGVuZm9yY2VzCisJICogdGhp
czsgcmUtY2hlY2sgaGVyZSBiZWZvcmUgdXNpbmcgdGhlIHZhbHVlcyBhcyBtZW1jcHkvTWVtU2V0
IGxlbmd0aHMKKwkgKiBvciBhcyB0aGUgZGVjb21wcmVzc29yIG91dHB1dCBjYXBhY2l0eS4KKwkg
Ki8KKwlpZiAoYmtwYi0+aG9sZV9vZmZzZXQgPiBCTENLU1ogfHwKKwkJYmtwYi0+aG9sZV9sZW5n
dGggPiBCTENLU1ogLSBia3BiLT5ob2xlX29mZnNldCkKKwl7CisJCXJlcG9ydF9pbnZhbGlkX3Jl
Y29yZChyZWNvcmQsCisJCQkJCQkJICAiY291bGQgbm90IHJlc3RvcmUgaW1hZ2UgYXQgJVgvJVgg
d2l0aCBpbnZhbGlkIHN0YXRlLCBibG9jayAlZCIsCisJCQkJCQkJICBMU05fRk9STUFUX0FSR1Mo
cmVjb3JkLT5SZWFkUmVjUHRyKSwKKwkJCQkJCQkgIGJsb2NrX2lkKTsKKwkJcmV0dXJuIGZhbHNl
OworCX0KKwogCWlmIChCS1BJTUFHRV9DT01QUkVTU0VEKGJrcGItPmJpbWdfaW5mbykpCiAJewog
CQkvKiBJZiBhIGJhY2t1cCBibG9jayBpbWFnZSBpcyBjb21wcmVzc2VkLCBkZWNvbXByZXNzIGl0
ICovCi0tIAoyLjM5LjIK

--_002_cc0a5886d2a74f99ae58c1647ea5ed8bftdataru_--