BUG #19613: pg_restore: several SEGVs in ReadToc() in pg_backup_archiver.c

Hi, Ilia!

Thanks for the report.

The attached patch
adds a small ReadRequiredStr() helper for fields that must be present in
a valid archive, and keeps ReadStr() for the nullable cases. The error
names the missing field and follows the existing "perhaps a corrupt TOC"
wording.

Verified with the reporter's base64 reproducer under AddressSanitizer:
pg_restore exits with a TOC error and no ASan SEGV.

вт, 11 авг. 2026 г. в 09:55, PG Bug reporting form <noreply@postgresql.org>:

The following bug has been logged on the website:

Bug reference: 19613
Logged by: Ilia Kashintsev
Email address: ilia.kashintsev@gmail.com
PostgreSQL version: 19beta2
Operating system: Ubuntu 24.04.4 LTS
Description:

Hello maintainers!
I have found several SEGVs on unknown address in ReadToc().

They occur because return value of numerous ReadStr(AH) calls is never
checked, with sscanf() or strcmp() being called on tmp == NULL.

For example pg_backup_archiver:2738-2739:

tmp = ReadStr(AH);
sscanf(tmp, "%u", &te->catalogId.tableoid); <------

Steps to reproduce:

1) Build the project with ASAN;
sudo mkdir -p /builds2
sudo chown "$(whoami)" /builds2

mkdir -p asan_build
cd asan_build
export CC=clang
export CXX=clang++
export CFLAGS="-O1 -g -fsanitize=address -fno-omit-frame-pointer"
export CXXFLAGS="-O1 -g -fsanitize=address -fno-omit-frame-pointer"
export LDFLAGS="-fsanitize=address"

../postgres/configure --prefix=/builds2/pg-asan
make -j
sudo make install

2) Run the example:

echo 'UEdETVABDDABMAEwMDAwMDAwMDAwMDAwMDAwMDAwMDAwADAAMDAwMDA=' | base64 -d

inp.bin

/builds2/pg-asan/bin/pg_restore -f dump.sql inp.bin

Sanitizer output:
AddressSanitizer:DEADLYSIGNAL
=================================================================
==247028==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000
(pc 0x778e90f7995d bp 0x7ffe3361c840 sp 0x7ffe3361c818 T0)
==247028==The signal is caused by a READ memory access.
==247028==Hint: address points to the zero page.
#0 0x778e90f7995d in __strlen_avx2
string/../sysdeps/x86_64/multiarch/strlen-avx2.S:76
#1 0x778e90e853d4 in _IO_str_init_static_internal libio/strops.c:41:11
#2 0x778e90e4dd10 in _IO_strfile_read
stdio-common/../libio/strfile.h:90:3
#3 0x778e90e4dd10 in __isoc23_vsscanf
stdio-common/isoc23_vsscanf.c:24:13
#4 0x62420175134d in __isoc23_sscanf
(/builds2/pg-asan/bin/pg_restore+0x6734d) (BuildId:
b947abf32a751f35042d5aa2948e2318357a75a0)
#5 0x62420182553e in ReadToc

/home/reproduce/asan_build/../postgres/src/bin/pg_dump/pg_backup_archiver.c:2739:4
#6 0x62420182c929 in InitArchiveFmt_Custom

/home/reproduce/asan_build/../postgres/src/bin/pg_dump/pg_backup_custom.c:180:3
#7 0x6242018164f1 in _allocAH

/home/reproduce/asan_build/../postgres/src/bin/pg_dump/pg_backup_archiver.c:2470:4
#8 0x624201816b1e in OpenArchive

/home/reproduce/asan_build/../postgres/src/bin/pg_dump/pg_backup_archiver.c:254:7
#9 0x624201807641 in main
/home/reproduce/asan_build/../postgres/src/bin/pg_dump/pg_restore.c:488:7
#10 0x778e90e181c9 in __libc_start_call_main
csu/../sysdeps/nptl/libc_start_call_main.h:58:16
#11 0x778e90e1828a in __libc_start_main csu/../csu/libc-start.c:360:3
#12 0x62420172c984 in _start (/builds2/pg-asan/bin/pg_restore+0x42984)
(BuildId: b947abf32a751f35042d5aa2948e2318357a75a0)

AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV
string/../sysdeps/x86_64/multiarch/strlen-avx2.S:76 in __strlen_avx2
==247028==ABORTING

Suggested fix:
Checking the return value of ReadStr resolves the issue.

diff --git a/src/bin/pg_dump/pg_backup_archiver.c
b/src/bin/pg_dump/pg_backup_archiver.c
index d7da3fc..3e9ac90 100644
--- a/src/bin/pg_dump/pg_backup_archiver.c
+++ b/src/bin/pg_dump/pg_backup_archiver.c
@@ -2736,18 +2736,26 @@ ReadToc(ArchiveHandle *AH)
if (AH->version >= K_VERS_1_8)
{
tmp = ReadStr(AH);
+                       if (tmp == NULL)
+                               pg_fatal("corrupt TOC: missing tableoid");
sscanf(tmp, "%u", &te->catalogId.tableoid);
free(tmp);
}
else
te->catalogId.tableoid = InvalidOid;
tmp = ReadStr(AH);
+               if (tmp == NULL)
+                       pg_fatal("corrupt TOC: missing oid");
sscanf(tmp, "%u", &te->catalogId.oid);
free(tmp);

te->tag = ReadStr(AH);
-               te->desc = ReadStr(AH);
+               if (te->tag == NULL)
+                       pg_fatal("corrupt TOC: missing entry tag");

+               te->desc = ReadStr(AH);
+               if (te->desc == NULL)
+                       pg_fatal("corrupt TOC: missing entry description");
if (AH->version >= K_VERS_1_11)
{
te->section = ReadInt(AH);
@@ -2804,6 +2812,8 @@ ReadToc(ArchiveHandle *AH)
{
tmp = ReadStr(AH);

+                       if (tmp == NULL)
+                               pg_fatal("corrupt TOC: missing WITH OIDS
marker");
if (strcmp(tmp, "true") == 0)
is_supported = false;

--
Regards,
Rachitskiy Andrey

--
Regards,
Rachitskiy Andrey

Hi Andrey,

The attached patch adds a small ReadRequiredStr() helper for fields
that must be present in a valid archive, and keeps ReadStr() for the
nullable cases.

I've read the patch. The distinction between five required strings and
the remaining nullable fields looks correct to me.

Could we add a regression test? The reporter's reproducer is short and
looks cool, needs no server, and could fit naturally in t/001_basic.pl. A
command_fails_like() check for the new "missing table OID in TOC" error
would prove that the original crash is fixed and protect the new helper.

It might be worth covering the other four call sites too, but I would
consider the original reproducer sufficient for this patch.

I found one earlier report of this exact failure, from 2021 [0]/messages/by-id/CF8A97DB-240C-4E9E-826D-743D6AD1C27B@legalserver.org. In that
case, ssh -t put pg_dump through a pseudo-terminal and corrupted the
binary stream; pg_restore then crashed at the same sscanf(NULL) in
ReadToc(). The NULL case came up again in a 2022 discussion [1]/messages/by-id/70019E5D-A6AB-43BA-84F9-D36EB8C678B6@yesql.se, but was
left unfixed partly because valid pg_dump output cannot reach it.

So this is an old defect with at least one real user report, not only a
synthetic malformed input. That seems like a good reason to back-patch
the fix to all supported branches. The helper keeps the change small,
and valid archives retain exactly the same behavior.

Apart from the missing test, the fix looks good to me.

Thank you!

Best regards, Andrey Borodin.

[0]: /messages/by-id/CF8A97DB-240C-4E9E-826D-743D6AD1C27B@legalserver.org
[1]: /messages/by-id/70019E5D-A6AB-43BA-84F9-D36EB8C678B6@yesql.se

пт, 14 авг. 2026 г. в 16:58, Andrey Borodin <x4mmm@yandex-team.ru>:

Could we add a regression test? The reporter's reproducer is short and
looks cool, needs no server, and could fit naturally in t/001_basic.pl. A
command_fails_like() check for the new "missing table OID in TOC" error
would prove that the original crash is fixed and protect the new helper.

It might be worth covering the other four call sites too, but I would
consider the original reproducer sufficient for this patch.

Hi Andrey!

Thanks for the review.

I added a regression test, attached as v2. It goes into
src/bin/pg_dump/t/001_basic.pl next to the other checks that need no
server. Rather than just the reporter's reproducer, it now covers every
ReadRequiredStr() call site in ReadToc(). The test builds a handful of
minimal custom-format archives, each with exactly one required TOC field
encoded as a NULL string (table OID, OID, entry tag, entry description
and the WITH OIDS marker), and uses command_fails_like() to require the
matching "missing <field> in TOC" error. The archive bytes are produced
by two tiny helpers.

The test does not depend on anything version-specific. It writes an
archive with a 1.12 header, a version every supported branch still reads,
and ReadToc() consumes those TOC fields in the same order on all of them.
The nullable fields it walks past are written as NULL, and the newer
fields (tableam, relkind) are gated on later versions and simply not
present at 1.12, so the same bytes exercise the same call sites from 14
to master. Valid archives are unaffected.

The test was checked on all releases from master to REL_14_STABLE

--
Regards,
Rachitskiy Andrey

Andrey Borodin <x4mmm@yandex-team.ru> writes:

Could we add a regression test?

-1 ... I think the engineering, maintenance, and runtime cost of a
test like this would never pay for itself. The thing you would wish
that a regression test could catch is adding a new TOC field that is
effectively required while failing to use ReadRequiredStr for it.
Which a test like this wouldn't.

regards, tom lane

пт, 14 авг. 2026 г. в 21:33, Tom Lane <tgl@sss.pgh.pa.us>:

The thing you would wish
that a regression test could catch is adding a new TOC field that is
effectively required while failing to use ReadRequiredStr for it.

Hi, Tom!

I've been thinking about it. And writing a universal test that would catch
new fields is quite difficult.
The current one I wrote only catches ReadRequiredStr.
So I suggest taking the fix without a test.

--
Regards,
Rachitskiy Andrey

The thing you would wish
that a regression test could catch is adding a new TOC field that is
effectively required while failing to use ReadRequiredStr for it.
Which a test like this wouldn't.

Agreed. I was thinking about proving the current fix, but that is not
the likely future regression.

I also considered constructing an archive with each string field set
to NULL in turn. That still requires the test to maintain its own list
of TOC fields, so a newly added field would not automatically be
covered.

We could encode the invariant in the API by making callers explicitly
choose ReadRequiredStr() or ReadNullableStr(), but that is quite a bit
more changes for a small back-patchable(?) fix.

Thank you!

Best regards, Andrey Borodin.