From 45cc3b63a3680066ab198b1a0141a9c9fde9a619 Mon Sep 17 00:00:00 2001 From: David karapetyan Date: Sat, 25 Jul 2026 13:32:48 -0700 Subject: [PATCH] Fix XLogReader mishandling of oversized multi-page records. XLogRecordAssemble() refuses records larger than XLogRecordMaxSize, but the reader only checked a minimum xl_tot_len. A crafted multi-page record with xl_tot_len near UINT32_MAX could pass contrecord length checks, overflow allocate_recordbuf()'s uint32 size math, and corrupt memory during reassembly (or hit related asserts under cassert). Cap xl_tot_len at XLogRecordMaxSize in ValidXLogRecordHeader() and on the partial-header path before reassembly, and compute reassembly buffer sizes with size_t. Add a frontend test that feeds crafted multi-page WAL to the reader. --- src/backend/access/transam/xlogreader.c | 53 +++- src/test/modules/Makefile | 1 + src/test/modules/meson.build | 1 + src/test/modules/test_xlogreader/Makefile | 30 +++ src/test/modules/test_xlogreader/README | 35 +++ src/test/modules/test_xlogreader/meson.build | 34 +++ .../test_xlogreader/t/001_oversized_record.pl | 35 +++ .../test_xlogreader_oversized.c | 234 ++++++++++++++++++ 8 files changed, 417 insertions(+), 6 deletions(-) create mode 100644 src/test/modules/test_xlogreader/Makefile create mode 100644 src/test/modules/test_xlogreader/README create mode 100644 src/test/modules/test_xlogreader/meson.build create mode 100644 src/test/modules/test_xlogreader/t/001_oversized_record.pl create mode 100644 src/test/modules/test_xlogreader/test_xlogreader_oversized.c diff --git a/src/backend/access/transam/xlogreader.c b/src/backend/access/transam/xlogreader.c index 946907a..d1a7e38 100644 --- a/src/backend/access/transam/xlogreader.c +++ b/src/backend/access/transam/xlogreader.c @@ -186,20 +186,35 @@ XLogReaderFree(XLogReaderState *state) * abort records might need more space.) * * Note: This routine should *never* be called for xl_tot_len until the header - * of the record has been fully validated. + * of the record has been fully validated (including the XLogRecordMaxSize + * bound). Size math uses size_t so near-UINT32_MAX lengths cannot wrap to a + * small allocation. */ static void allocate_recordbuf(XLogReaderState *state, uint32 reclength) { - uint32 newSize = reclength; + size_t newSize = (size_t) reclength; + size_t minsize = (size_t) 5 * Max(BLCKSZ, XLOG_BLCKSZ); - newSize += XLOG_BLCKSZ - (newSize % XLOG_BLCKSZ); - newSize = Max(newSize, 5 * Max(BLCKSZ, XLOG_BLCKSZ)); + /* + * Round up to a multiple of XLOG_BLCKSZ. When already aligned (including + * zero), advance by a full page — same as the historical uint32 formula. + */ + newSize += (size_t) XLOG_BLCKSZ - (newSize % (size_t) XLOG_BLCKSZ); + if (newSize < minsize) + newSize = minsize; + + /* + * Callers must reject xl_tot_len > XLogRecordMaxSize first. With that + * bound, the rounded size always fits in uint32 (and palloc's limit). + */ + Assert(newSize <= (size_t) XLogRecordMaxSize + (size_t) XLOG_BLCKSZ); + Assert(newSize <= (size_t) PG_UINT32_MAX); if (state->readRecordBuf) pfree(state->readRecordBuf); state->readRecordBuf = (char *) palloc(newSize); - state->readRecordBufSize = newSize; + state->readRecordBufSize = (uint32) newSize; } /* @@ -664,7 +679,11 @@ restart: } else { - /* There may be no next page if it's too small. */ + /* + * There may be no next page if it's too small. Cap xl_tot_len before + * contrecord reassembly so we never allocate or copy based on a + * garbage length from a recycled page. + */ if (total_len < SizeOfXLogRecord) { report_invalid_record(state, @@ -673,6 +692,14 @@ restart: (uint32) SizeOfXLogRecord, total_len); goto err; } + if (total_len > XLogRecordMaxSize) + { + report_invalid_record(state, + "invalid record length at %X/%08X: expected at most %u, got %u", + LSN_FORMAT_ARGS(RecPtr), + XLogRecordMaxSize, total_len); + goto err; + } /* We'll validate the header once we have the next page. */ gotheader = false; } @@ -1148,6 +1175,20 @@ ValidXLogRecordHeader(XLogReaderState *state, XLogRecPtr RecPtr, (uint32) SizeOfXLogRecord, record->xl_tot_len); return false; } + /* + * Symmetric with XLogRecordAssemble(): the reader must not attempt to + * reassemble or decode a record larger than XLogRecordMaxSize. Without + * this bound, a crafted multi-page xl_tot_len near UINT32_MAX can overflow + * allocate_recordbuf()'s size math and corrupt memory during reassembly. + */ + if (record->xl_tot_len > XLogRecordMaxSize) + { + report_invalid_record(state, + "invalid record length at %X/%08X: expected at most %u, got %u", + LSN_FORMAT_ARGS(RecPtr), + XLogRecordMaxSize, record->xl_tot_len); + return false; + } if (!RmgrIdIsValid(record->xl_rmid)) { report_invalid_record(state, diff --git a/src/test/modules/Makefile b/src/test/modules/Makefile index 098bb81..4b59eff 100644 --- a/src/test/modules/Makefile +++ b/src/test/modules/Makefile @@ -53,6 +53,7 @@ SUBDIRS = \ test_shm_mq \ test_slru \ test_tidstore \ + test_xlogreader \ unsafe_tests \ worker_spi \ xid_wraparound diff --git a/src/test/modules/meson.build b/src/test/modules/meson.build index 4bca42b..effbaff 100644 --- a/src/test/modules/meson.build +++ b/src/test/modules/meson.build @@ -54,6 +54,7 @@ subdir('test_shmem') subdir('test_shm_mq') subdir('test_slru') subdir('test_tidstore') +subdir('test_xlogreader') subdir('typcache') subdir('unsafe_tests') subdir('worker_spi') diff --git a/src/test/modules/test_xlogreader/Makefile b/src/test/modules/test_xlogreader/Makefile new file mode 100644 index 0000000..bf9f574 --- /dev/null +++ b/src/test/modules/test_xlogreader/Makefile @@ -0,0 +1,30 @@ +# src/test/modules/test_xlogreader/Makefile + +PGFILEDESC = "test_xlogreader - XLogReader validation tests" + +PROGRAM = test_xlogreader_oversized +OBJS = $(WIN32RES) test_xlogreader_oversized.o xlogreader.o + +# Frontend build of backend xlogreader.c (same pattern as pg_waldump) +override CPPFLAGS := -DFRONTEND $(CPPFLAGS) +PG_CPPFLAGS = -I$(libpq_srcdir) +PG_LIBS_INTERNAL += $(libpq_pgport) + +NO_INSTALL = 1 +TAP_TESTS = 1 + +EXTRA_CLEAN = xlogreader.c + +ifdef USE_PGXS +PG_CONFIG = pg_config +PGXS := $(shell $(PG_CONFIG) --pgxs) +include $(PGXS) +else +subdir = src/test/modules/test_xlogreader +top_builddir = ../../../.. +include $(top_builddir)/src/Makefile.global +include $(top_srcdir)/contrib/contrib-global.mk +endif + +xlogreader.c: % : $(top_srcdir)/src/backend/access/transam/% + rm -f $@ && $(LN_S) $< . diff --git a/src/test/modules/test_xlogreader/README b/src/test/modules/test_xlogreader/README new file mode 100644 index 0000000..02eecfc --- /dev/null +++ b/src/test/modules/test_xlogreader/README @@ -0,0 +1,35 @@ +test_xlogreader +=============== + +Frontend tests for XLogReader validation of pathological multi-page WAL. + +test_xlogreader_oversized +------------------------- + +Constructs an in-memory multi-page WAL record with: + +* xl_tot_len = 0xFFFFF000 (near UINT32_MAX, far above XLogRecordMaxSize) +* matching XLP_FIRST_IS_CONTRECORD / xlp_rem_len on continuation pages + +and feeds it to XLogReader. + +Correct behavior + XLogReadRecord() returns NULL with an error; process exits 0. + +Buggy behavior (current mainline as of this test) + allocate_recordbuf() rounds the huge length with uint32 arithmetic and + wraps to a ~40kB buffer. On the next continuation page, reassembly hits: + + Assert(gotlen <= lengthof(save_copy)); /* save_copy is 2 * XLOG_BLCKSZ */ + + under USE_ASSERT_CHECKING (abort), or stack/heap overflow without asserts. + +Build / run +----------- + + make -C src/test/modules/test_xlogreader + ./src/test/modules/test_xlogreader/test_xlogreader_oversized + +With TAP enabled (--enable-tap-tests): + + make -C src/test/modules/test_xlogreader check diff --git a/src/test/modules/test_xlogreader/meson.build b/src/test/modules/test_xlogreader/meson.build new file mode 100644 index 0000000..acc1e92 --- /dev/null +++ b/src/test/modules/test_xlogreader/meson.build @@ -0,0 +1,34 @@ +# Copyright (c) 2026, PostgreSQL Global Development Group + +test_xlogreader_sources = files( + 'test_xlogreader_oversized.c', +) +test_xlogreader_sources += xlogreader_sources + +if host_system == 'windows' + test_xlogreader_sources += rc_bin_gen.process(win32ver_rc, extra_args: [ + '--NAME', 'test_xlogreader_oversized', + '--FILEDESC', 'test XLogReader oversized record handling',]) +endif + +test_xlogreader_oversized = executable('test_xlogreader_oversized', + test_xlogreader_sources, + dependencies: [frontend_code, libpq], + c_args: ['-DFRONTEND'], + kwargs: default_bin_args + { + 'install': false, + }, +) +testprep_targets += test_xlogreader_oversized + +tests += { + 'name': 'test_xlogreader', + 'sd': meson.current_source_dir(), + 'bd': meson.current_build_dir(), + 'tap': { + 'tests': [ + 't/001_oversized_record.pl', + ], + 'deps': [test_xlogreader_oversized], + }, +} diff --git a/src/test/modules/test_xlogreader/t/001_oversized_record.pl b/src/test/modules/test_xlogreader/t/001_oversized_record.pl new file mode 100644 index 0000000..ddde7d3 --- /dev/null +++ b/src/test/modules/test_xlogreader/t/001_oversized_record.pl @@ -0,0 +1,35 @@ +# Copyright (c) 2026, PostgreSQL Global Development Group + +# Expect XLogReader to reject multi-page records with xl_tot_len near +# UINT32_MAX (or above XLogRecordMaxSize) without crashing. + +use strict; +use warnings FATAL => 'all'; + +use PostgreSQL::Test::Utils; +use Test::More; + +my $exe = 'test_xlogreader_oversized'; + +note "running $exe (expects clean rejection of oversized multi-page WAL)"; + +# With the allocate_recordbuf overflow bug this often dies on SIGSEGV. +# After a proper max-length check it exits 0 and prints OK. +my $result = IPC::Run::run [ $exe ], '>', \my $stdout, '2>', \my $stderr; +my $exit = $? >> 8; +my $signal = $? & 127; + +if ($signal) +{ + fail("test_xlogreader_oversized died with signal $signal (likely heap overflow in allocate_recordbuf / reassembly)"); + diag("stdout: $stdout") if length $stdout; + diag("stderr: $stderr") if length $stderr; +} +else +{ + is($exit, 0, 'test_xlogreader_oversized exits 0'); + like($stdout, qr/^OK:/, 'reports clean rejection on stdout'); + is($stderr, '', 'no stderr on success'); +} + +done_testing(); diff --git a/src/test/modules/test_xlogreader/test_xlogreader_oversized.c b/src/test/modules/test_xlogreader/test_xlogreader_oversized.c new file mode 100644 index 0000000..43ba56f --- /dev/null +++ b/src/test/modules/test_xlogreader/test_xlogreader_oversized.c @@ -0,0 +1,234 @@ +/*------------------------------------------------------------------------- + * + * test_xlogreader_oversized.c + * Demonstrate that XLogReader fails to reject multi-page records whose + * xl_tot_len exceeds XLogRecordMaxSize, and that allocate_recordbuf() + * can overflow uint32 when rounding a near-UINT32_MAX length. + * + * Crafted multi-page WAL is fed through the frontend XLogReader. A + * correct reader must reject the record cleanly (return NULL with an + * error). Unfixed code reallocates with a wrapped size (~40kB) and + * then heap-overflows while reassembling continuation pages. + * + * Build and run (from a configured tree): + * make -C src/test/modules/test_xlogreader + * ./src/test/modules/test_xlogreader/test_xlogreader_oversized + * + * Expected with the bug: + * - with asserts: abort on + * Assert(gotlen <= lengthof(save_copy)) in XLogDecodeNextRecord + * (gotlen exceeds 2 pages while total_len still forces reallocation) + * - without asserts: stack/heap overflow around save_copy / + * allocate_recordbuf uint32 wrap + * Expected after fix: exit 0 and a rejection message on stdout. + * + *------------------------------------------------------------------------- + */ +#include "postgres_fe.h" + +#include + +#include "access/xlog_internal.h" +#include "access/xlogreader.h" +#include "access/xlogrecord.h" +#include "common/fe_memutils.h" + +/* Default segment size; only used for page-header cross-checks. */ +#define TEST_WAL_SEGSIZE (16 * 1024 * 1024) + +/* + * Enough pages for reassembly to reallocate and then overflow a 40kB + * buffer: first page + a few continuations. total_len claims ~4GB so + * the reader keeps consuming pages until it corrupts memory (bug) or + * rejects (fixed). + */ +#define TEST_NPAGES 16 + +/* Near UINT32_MAX so allocate_recordbuf() rounding wraps to 0. */ +#define TEST_XL_TOT_LEN ((uint32) 0xFFFFF000U) + +typedef struct TestWalState +{ + char pages[TEST_NPAGES][XLOG_BLCKSZ]; + int npages; +} TestWalState; + +static void +init_short_page_header(char *page, XLogRecPtr pageaddr, uint16 info, + uint32 rem_len) +{ + XLogPageHeader hdr = (XLogPageHeader) page; + + memset(page, 0, XLOG_BLCKSZ); + hdr->xlp_magic = XLOG_PAGE_MAGIC; + hdr->xlp_info = info; + hdr->xlp_tli = 1; + hdr->xlp_pageaddr = pageaddr; + hdr->xlp_rem_len = rem_len; +} + +static void +init_long_page_header(char *page, XLogRecPtr pageaddr, uint16 info, + uint32 rem_len) +{ + XLogLongPageHeader longhdr = (XLogLongPageHeader) page; + + init_short_page_header(page, pageaddr, info | XLP_LONG_HEADER, rem_len); + longhdr->xlp_sysid = 0; + longhdr->xlp_seg_size = TEST_WAL_SEGSIZE; + longhdr->xlp_xlog_blcksz = XLOG_BLCKSZ; +} + +/* + * Build a multi-page record starting at the first data byte after the long + * header on page 0. xl_tot_len is huge; continuation pages advertise + * matching xlp_rem_len values so the reader enters reassembly. + */ +static XLogRecPtr +build_oversized_multpage_wal(TestWalState *ws) +{ + XLogRecPtr page0 = 0; + XLogRecPtr recptr; + XLogRecord *rec; + char *payload; + uint32 gotlen; + int i; + int phd_long = SizeOfXLogLongPHD; + int phd_short = SizeOfXLogShortPHD; + + ws->npages = TEST_NPAGES; + + /* Page 0: long header + start of record */ + init_long_page_header(ws->pages[0], page0, 0, 0); + recptr = page0 + phd_long; + payload = ws->pages[0] + phd_long; + + /* Zero-fill usable area, then write the fixed-size record header. */ + memset(payload, 0, XLOG_BLCKSZ - phd_long); + rec = (XLogRecord *) payload; + rec->xl_tot_len = TEST_XL_TOT_LEN; + rec->xl_xid = 0; + rec->xl_prev = 0; /* randAccess accepts xl_prev < recptr */ + rec->xl_info = 0; + rec->xl_rmid = RM_XLOG_ID; + rec->xl_crc = 0; /* CRC never reached on the overflow path */ + + gotlen = XLOG_BLCKSZ - phd_long; + + /* Continuation pages with consistent xlp_rem_len */ + for (i = 1; i < TEST_NPAGES; i++) + { + XLogRecPtr pageaddr = (XLogRecPtr) i * XLOG_BLCKSZ; + uint32 rem = TEST_XL_TOT_LEN - gotlen; + int usable = XLOG_BLCKSZ - phd_short; + + init_short_page_header(ws->pages[i], pageaddr, + XLP_FIRST_IS_CONTRECORD, rem); + memset(ws->pages[i] + phd_short, 0xAB, usable); + gotlen += usable; + } + + return recptr; +} + +static int +test_page_read(XLogReaderState *xlogreader, XLogRecPtr targetPagePtr, + int reqLen, XLogRecPtr targetRecPtr, char *readBuf) +{ + TestWalState *ws = (TestWalState *) xlogreader->private_data; + uint64 idx; + + (void) reqLen; + (void) targetRecPtr; + + if (targetPagePtr % XLOG_BLCKSZ != 0) + return -1; + + idx = targetPagePtr / XLOG_BLCKSZ; + if (idx >= (uint64) ws->npages) + return -1; + + memcpy(readBuf, ws->pages[idx], XLOG_BLCKSZ); + xlogreader->seg.ws_tli = 1; + return XLOG_BLCKSZ; +} + +static void +test_segment_open(XLogReaderState *xlogreader, XLogSegNo nextSegNo, + TimeLineID *tli_p) +{ + (void) xlogreader; + (void) nextSegNo; + /* Keep TLI from caller / page_read. */ + if (tli_p && *tli_p == 0) + *tli_p = 1; + xlogreader->seg.ws_file = 0; /* dummy non -1 */ + xlogreader->seg.ws_segno = nextSegNo; + xlogreader->seg.ws_tli = 1; +} + +static void +test_segment_close(XLogReaderState *xlogreader) +{ + xlogreader->seg.ws_file = -1; +} + +int +main(int argc, char **argv) +{ + TestWalState ws; + XLogReaderState *state; + XLogReaderRoutine routine; + XLogRecPtr start; + XLogRecord *record; + char *errormsg = NULL; + + (void) argc; + (void) argv; + + memset(&ws, 0, sizeof(ws)); + start = build_oversized_multpage_wal(&ws); + + memset(&routine, 0, sizeof(routine)); + routine.page_read = test_page_read; + routine.segment_open = test_segment_open; + routine.segment_close = test_segment_close; + + state = XLogReaderAllocate(TEST_WAL_SEGSIZE, NULL, &routine, &ws); + if (state == NULL) + { + fprintf(stderr, "out of memory allocating XLogReader\n"); + return 2; + } + + /* + * Begin at the crafted record. A fixed reader must refuse xl_tot_len + * above XLogRecordMaxSize (or fail closed on size overflow) without + * scribbling past its reassembly buffer. + */ + XLogBeginRead(state, start); + record = XLogReadRecord(state, &errormsg); + + if (record != NULL) + { + fprintf(stderr, + "FAIL: oversized multi-page record was accepted " + "(xl_tot_len=%u XLogRecordMaxSize=%u)\n", + TEST_XL_TOT_LEN, (unsigned) XLogRecordMaxSize); + XLogReaderFree(state); + return 1; + } + + if (errormsg == NULL || errormsg[0] == '\0') + { + fprintf(stderr, + "FAIL: reader returned NULL without an error message\n"); + XLogReaderFree(state); + return 1; + } + + /* Clean rejection — this is the desired post-fix behavior. */ + printf("OK: rejected oversized record: %s\n", errormsg); + XLogReaderFree(state); + return 0; +} -- 2.43.0