agora inbox for pgsql-bugs@postgresql.org
help / color / mirror / Atom feedBUG #19613: pg_restore: several SEGVs in ReadToc() in pg_backup_archiver.c
7+ messages / 4 participants
[nested] [flat]
* BUG #19613: pg_restore: several SEGVs in ReadToc() in pg_backup_archiver.c
@ 2026-08-07 13:37 PG Bug reporting form <noreply@postgresql.org>
2026-08-11 07:05 ` Re: BUG #19613: pg_restore: several SEGVs in ReadToc() in pg_backup_archiver.c Andrey Rachitskiy <pl0h0yp1@gmail.com>
0 siblings, 1 reply; 7+ messages in thread
From: PG Bug reporting form @ 2026-08-07 13:37 UTC (permalink / raw)
To: pgsql-bugs@lists.postgresql.org; +Cc: ilia.kashintsev@gmail.com
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;
^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: BUG #19613: pg_restore: several SEGVs in ReadToc() in pg_backup_archiver.c
2026-08-07 13:37 BUG #19613: pg_restore: several SEGVs in ReadToc() in pg_backup_archiver.c PG Bug reporting form <noreply@postgresql.org>
@ 2026-08-11 07:05 ` Andrey Rachitskiy <pl0h0yp1@gmail.com>
2026-08-14 11:58 ` Re: BUG #19613: pg_restore: several SEGVs in ReadToc() in pg_backup_archiver.c Andrey Borodin <x4mmm@yandex-team.ru>
0 siblings, 1 reply; 7+ messages in thread
From: Andrey Rachitskiy @ 2026-08-11 07:05 UTC (permalink / raw)
To: ilia.kashintsev@gmail.com; pgsql-bugs@lists.postgresql.org
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
Attachments:
[text/x-patch] 0001-Fix-SEGV-in-ReadToc-on-NULL-ReadStr-results.patch (2.9K, ../../CAB8bMisXgPyHXQeiPTbemp_uANRXPUgyytbDsMfywJGyAqvDOA@mail.gmail.com/3-0001-Fix-SEGV-in-ReadToc-on-NULL-ReadStr-results.patch)
download | inline diff:
From 5fc3305bf2e41212ccab9c6475c414107964e6c8 Mon Sep 17 00:00:00 2001
From: Andrey Rachitskiy <pl0h0yp1@gmail.com>
Date: Tue, 11 Aug 2026 11:48:36 +0500
Subject: [PATCH] Fix SEGV in ReadToc on NULL ReadStr results
ReadStr() may return NULL for a negative length word. That is valid for
nullable TOC fields and the dependency-list terminator, but ReadToc()
passed such pointers to sscanf()/strcmp() for fields that are always
present in a valid archive. Add ReadRequiredStr() for those sites so
pg_restore reports a corrupt TOC instead of crashing.
Bug: #19613
Author: Andrey Rachitskiy <pl0h0yp1@gmail.com>
Reported-by: Ilia Kashintsev <ilia.kashintsev@gmail.com>
---
src/bin/pg_dump/pg_backup_archiver.c | 30 +++++++++++++++++++++++-----
1 file changed, 25 insertions(+), 5 deletions(-)
diff --git a/src/bin/pg_dump/pg_backup_archiver.c b/src/bin/pg_dump/pg_backup_archiver.c
index d7da3fc4325..8b5ec260b86 100644
--- a/src/bin/pg_dump/pg_backup_archiver.c
+++ b/src/bin/pg_dump/pg_backup_archiver.c
@@ -85,6 +85,7 @@ static int _discoverArchiveFormat(ArchiveHandle *AH);
static int RestoringToDB(ArchiveHandle *AH);
static void dump_lo_buf(ArchiveHandle *AH);
+static char *ReadRequiredStr(ArchiveHandle *AH, const char *fieldname);
static void dumpTimestamp(ArchiveHandle *AH, const char *msg, time_t tim);
static void SetOutput(ArchiveHandle *AH, const char *filename,
const pg_compress_specification compression_spec);
@@ -2227,6 +2228,25 @@ ReadStr(ArchiveHandle *AH)
return buf;
}
+/*
+ * Like ReadStr(), but require a non-NULL string.
+ *
+ * A negative length is a valid NULL encoding in the archive format
+ * (nullable TOC fields, dependency-list terminator). Use this only for
+ * fields that a valid dump always writes as a real string. Getting NULL
+ * there means the TOC is incomplete or corrupt.
+ */
+static char *
+ReadRequiredStr(ArchiveHandle *AH, const char *fieldname)
+{
+ char *result;
+
+ result = ReadStr(AH);
+ if (result == NULL)
+ pg_fatal("missing %s in TOC -- perhaps a corrupt TOC", fieldname);
+ return result;
+}
+
static bool
_fileExistsInDirectory(const char *dir, const char *filename)
{
@@ -2735,18 +2755,18 @@ ReadToc(ArchiveHandle *AH)
if (AH->version >= K_VERS_1_8)
{
- tmp = ReadStr(AH);
+ tmp = ReadRequiredStr(AH, "table OID");
sscanf(tmp, "%u", &te->catalogId.tableoid);
free(tmp);
}
else
te->catalogId.tableoid = InvalidOid;
- tmp = ReadStr(AH);
+ tmp = ReadRequiredStr(AH, "OID");
sscanf(tmp, "%u", &te->catalogId.oid);
free(tmp);
- te->tag = ReadStr(AH);
- te->desc = ReadStr(AH);
+ te->tag = ReadRequiredStr(AH, "entry tag");
+ te->desc = ReadRequiredStr(AH, "entry description");
if (AH->version >= K_VERS_1_11)
{
@@ -2802,7 +2822,7 @@ ReadToc(ArchiveHandle *AH)
is_supported = false;
else
{
- tmp = ReadStr(AH);
+ tmp = ReadRequiredStr(AH, "WITH OIDS marker");
if (strcmp(tmp, "true") == 0)
is_supported = false;
--
2.53.0
^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: BUG #19613: pg_restore: several SEGVs in ReadToc() in pg_backup_archiver.c
2026-08-07 13:37 BUG #19613: pg_restore: several SEGVs in ReadToc() in pg_backup_archiver.c PG Bug reporting form <noreply@postgresql.org>
2026-08-11 07:05 ` Re: BUG #19613: pg_restore: several SEGVs in ReadToc() in pg_backup_archiver.c Andrey Rachitskiy <pl0h0yp1@gmail.com>
@ 2026-08-14 11:58 ` Andrey Borodin <x4mmm@yandex-team.ru>
2026-08-14 13:59 ` Re: BUG #19613: pg_restore: several SEGVs in ReadToc() in pg_backup_archiver.c Andrey Rachitskiy <pl0h0yp1@gmail.com>
2026-08-14 16:33 ` Re: BUG #19613: pg_restore: several SEGVs in ReadToc() in pg_backup_archiver.c Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 2 replies; 7+ messages in thread
From: Andrey Borodin @ 2026-08-14 11:58 UTC (permalink / raw)
To: Andrey Rachitskiy <pl0h0yp1@gmail.com>; +Cc: ilia.kashintsev@gmail.com, PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>
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]. 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], 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] https://postgr.es/m/CF8A97DB-240C-4E9E-826D-743D6AD1C27B@legalserver.org
[1] https://postgr.es/m/70019E5D-A6AB-43BA-84F9-D36EB8C678B6@yesql.se
^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: BUG #19613: pg_restore: several SEGVs in ReadToc() in pg_backup_archiver.c
2026-08-07 13:37 BUG #19613: pg_restore: several SEGVs in ReadToc() in pg_backup_archiver.c PG Bug reporting form <noreply@postgresql.org>
2026-08-11 07:05 ` Re: BUG #19613: pg_restore: several SEGVs in ReadToc() in pg_backup_archiver.c Andrey Rachitskiy <pl0h0yp1@gmail.com>
2026-08-14 11:58 ` Re: BUG #19613: pg_restore: several SEGVs in ReadToc() in pg_backup_archiver.c Andrey Borodin <x4mmm@yandex-team.ru>
@ 2026-08-14 13:59 ` Andrey Rachitskiy <pl0h0yp1@gmail.com>
1 sibling, 0 replies; 7+ messages in thread
From: Andrey Rachitskiy @ 2026-08-14 13:59 UTC (permalink / raw)
To: Andrey Borodin <x4mmm@yandex-team.ru>; +Cc: ilia.kashintsev@gmail.com, PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>
пт, 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
Attachments:
[text/x-patch] v2-0001-Fix-SEGV-in-ReadToc-on-NULL-ReadStr-results.patch (6.1K, ../../CAB8bMitfbp9pD_7CLxfWHhHDWj3JQicfPKayGWKqoHgcchf9SA@mail.gmail.com/3-v2-0001-Fix-SEGV-in-ReadToc-on-NULL-ReadStr-results.patch)
download | inline diff:
From bc353e73aa64e3fc950ab6eb68271c0e90b925b6 Mon Sep 17 00:00:00 2001
From: Andrey Rachitskiy <pl0h0yp1@gmail.com>
Date: Fri, 14 Aug 2026 18:21:08 +0500
Subject: [PATCH v2] Fix SEGV in ReadToc on NULL ReadStr results
ReadStr() may return NULL for a negative length word. That is valid for
nullable TOC fields and the dependency-list terminator, but ReadToc()
passed such pointers to sscanf()/strcmp() for fields that are always
present in a valid archive. Add ReadRequiredStr() for those sites so
pg_restore reports a corrupt TOC instead of crashing.
Add a regression test in src/bin/pg_dump/t/001_basic.pl that builds
minimal custom-format archives, each with one required TOC field encoded
as a NULL string, and checks that pg_restore reports a corrupt TOC for
every ReadRequiredStr() call site.
Bug: #19613
Author: Andrey Rachitskiy <pl0h0yp1@gmail.com>
Reported-by: Ilia Kashintsev <ilia.kashintsev@gmail.com>
---
src/bin/pg_dump/pg_backup_archiver.c | 30 ++++++++++--
src/bin/pg_dump/t/001_basic.pl | 69 ++++++++++++++++++++++++++++
2 files changed, 94 insertions(+), 5 deletions(-)
diff --git a/src/bin/pg_dump/pg_backup_archiver.c b/src/bin/pg_dump/pg_backup_archiver.c
index d7da3fc4325..8b5ec260b86 100644
--- a/src/bin/pg_dump/pg_backup_archiver.c
+++ b/src/bin/pg_dump/pg_backup_archiver.c
@@ -85,6 +85,7 @@ static int _discoverArchiveFormat(ArchiveHandle *AH);
static int RestoringToDB(ArchiveHandle *AH);
static void dump_lo_buf(ArchiveHandle *AH);
+static char *ReadRequiredStr(ArchiveHandle *AH, const char *fieldname);
static void dumpTimestamp(ArchiveHandle *AH, const char *msg, time_t tim);
static void SetOutput(ArchiveHandle *AH, const char *filename,
const pg_compress_specification compression_spec);
@@ -2227,6 +2228,25 @@ ReadStr(ArchiveHandle *AH)
return buf;
}
+/*
+ * Like ReadStr(), but require a non-NULL string.
+ *
+ * A negative length is a valid NULL encoding in the archive format
+ * (nullable TOC fields, dependency-list terminator). Use this only for
+ * fields that a valid dump always writes as a real string. Getting NULL
+ * there means the TOC is incomplete or corrupt.
+ */
+static char *
+ReadRequiredStr(ArchiveHandle *AH, const char *fieldname)
+{
+ char *result;
+
+ result = ReadStr(AH);
+ if (result == NULL)
+ pg_fatal("missing %s in TOC -- perhaps a corrupt TOC", fieldname);
+ return result;
+}
+
static bool
_fileExistsInDirectory(const char *dir, const char *filename)
{
@@ -2735,18 +2755,18 @@ ReadToc(ArchiveHandle *AH)
if (AH->version >= K_VERS_1_8)
{
- tmp = ReadStr(AH);
+ tmp = ReadRequiredStr(AH, "table OID");
sscanf(tmp, "%u", &te->catalogId.tableoid);
free(tmp);
}
else
te->catalogId.tableoid = InvalidOid;
- tmp = ReadStr(AH);
+ tmp = ReadRequiredStr(AH, "OID");
sscanf(tmp, "%u", &te->catalogId.oid);
free(tmp);
- te->tag = ReadStr(AH);
- te->desc = ReadStr(AH);
+ te->tag = ReadRequiredStr(AH, "entry tag");
+ te->desc = ReadRequiredStr(AH, "entry description");
if (AH->version >= K_VERS_1_11)
{
@@ -2802,7 +2822,7 @@ ReadToc(ArchiveHandle *AH)
is_supported = false;
else
{
- tmp = ReadStr(AH);
+ tmp = ReadRequiredStr(AH, "WITH OIDS marker");
if (strcmp(tmp, "true") == 0)
is_supported = false;
diff --git a/src/bin/pg_dump/t/001_basic.pl b/src/bin/pg_dump/t/001_basic.pl
index b2558046224..840a1313376 100644
--- a/src/bin/pg_dump/t/001_basic.pl
+++ b/src/bin/pg_dump/t/001_basic.pl
@@ -250,4 +250,73 @@ command_fails_like(
'pg_dumpall: option --exclude-database cannot be used together with -g/--globals-only'
);
+#########################################
+# Corrupt archive checks
+
+# ReadToc() reads several TOC fields that a valid dump always stores as a real
+# string. A NULL there (encoded as a negative length) used to reach sscanf()
+# or strcmp() and crash pg_restore (bug #19613). Build minimal custom-format
+# archives, each with one such field encoded as NULL, and check that pg_restore
+# reports a corrupt TOC instead of crashing.
+
+# Encode an integer as the archive does: a sign byte plus intSize magnitude
+# bytes (we set intSize = 1 in the header, so one byte covers these values).
+sub toc_int
+{
+ my ($v) = @_;
+ return pack('CC', ($v < 0 ? 1 : 0), abs($v) & 0xFF);
+}
+
+# Encode a string, or a NULL field (negative length) when undef.
+sub toc_str
+{
+ my ($s) = @_;
+ return defined $s ? toc_int(length $s) . $s : toc_int(-1);
+}
+
+# Custom-format header (version 1.12, format 1, intSize/offSize 1) accepted by
+# ReadHead(), then a TOC announcing a single entry.
+my $toc_header = 'PGDMP' . pack('CCC', 1, 12, 0) . pack('CCC', 1, 1, 1);
+$toc_header .= toc_int(0); # compression level (none)
+$toc_header .= toc_int(0) x 3; # createDate: sec, min, hour
+$toc_header .= toc_int(1) . toc_int(0) . toc_int(100); # mday, mon, year
+$toc_header .= toc_int(0); # createDate: isdst
+$toc_header .= toc_str(undef) x 3; # dbname, remote and dump version
+$toc_header .= toc_int(1); # tocCount
+
+# Every entry starts with dumpId and hadDumper.
+my $toc_entry = toc_int(1) . toc_int(0);
+
+# Each case supplies valid required strings up to the field under test, which is
+# then left NULL. The names match the ReadRequiredStr() call sites in ReadToc().
+my @toc_cases = (
+ [ 'table OID', '' ],
+ [ 'OID', toc_str('0') ],
+ [ 'entry tag', toc_str('0') . toc_str('0') ],
+ [ 'entry description', toc_str('0') . toc_str('0') . toc_str('t') ],
+ [
+ 'WITH OIDS marker',
+ toc_str('0') . toc_str('0') . toc_str('t') . toc_str('d')
+ . toc_int(0) # section
+ . (toc_str(undef) x 6) # defn, dropStmt, copyStmt, namespace,
+ # tablespace, owner (all nullable)
+ ],
+);
+
+my $toc_case_no = 0;
+foreach my $toc_case (@toc_cases)
+{
+ my ($field, $prefix) = @$toc_case;
+ my $file = "$tempdir/corrupt_toc_${toc_case_no}.dump";
+ open my $fh, '>:raw', $file or die "could not create $file: $!";
+ print $fh $toc_header . $toc_entry . $prefix . toc_str(undef);
+ close $fh;
+
+ command_fails_like(
+ [ 'pg_restore', '-l', $file ],
+ qr/\Qpg_restore: error: missing $field in TOC -- perhaps a corrupt TOC\E/,
+ "pg_restore: NULL $field in TOC reported as corrupt");
+ $toc_case_no++;
+}
+
done_testing();
--
2.53.0
^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: BUG #19613: pg_restore: several SEGVs in ReadToc() in pg_backup_archiver.c
2026-08-07 13:37 BUG #19613: pg_restore: several SEGVs in ReadToc() in pg_backup_archiver.c PG Bug reporting form <noreply@postgresql.org>
2026-08-11 07:05 ` Re: BUG #19613: pg_restore: several SEGVs in ReadToc() in pg_backup_archiver.c Andrey Rachitskiy <pl0h0yp1@gmail.com>
2026-08-14 11:58 ` Re: BUG #19613: pg_restore: several SEGVs in ReadToc() in pg_backup_archiver.c Andrey Borodin <x4mmm@yandex-team.ru>
@ 2026-08-14 16:33 ` Tom Lane <tgl@sss.pgh.pa.us>
2026-08-14 18:02 ` Re: BUG #19613: pg_restore: several SEGVs in ReadToc() in pg_backup_archiver.c Andrey Rachitskiy <pl0h0yp1@gmail.com>
2026-08-16 15:03 ` Re: BUG #19613: pg_restore: several SEGVs in ReadToc() in pg_backup_archiver.c Andrey Borodin <x4mmm@yandex-team.ru>
1 sibling, 2 replies; 7+ messages in thread
From: Tom Lane @ 2026-08-14 16:33 UTC (permalink / raw)
To: Andrey Borodin <x4mmm@yandex-team.ru>; +Cc: Andrey Rachitskiy <pl0h0yp1@gmail.com>; ilia.kashintsev@gmail.com, PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>
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
^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: BUG #19613: pg_restore: several SEGVs in ReadToc() in pg_backup_archiver.c
2026-08-07 13:37 BUG #19613: pg_restore: several SEGVs in ReadToc() in pg_backup_archiver.c PG Bug reporting form <noreply@postgresql.org>
2026-08-11 07:05 ` Re: BUG #19613: pg_restore: several SEGVs in ReadToc() in pg_backup_archiver.c Andrey Rachitskiy <pl0h0yp1@gmail.com>
2026-08-14 11:58 ` Re: BUG #19613: pg_restore: several SEGVs in ReadToc() in pg_backup_archiver.c Andrey Borodin <x4mmm@yandex-team.ru>
2026-08-14 16:33 ` Re: BUG #19613: pg_restore: several SEGVs in ReadToc() in pg_backup_archiver.c Tom Lane <tgl@sss.pgh.pa.us>
@ 2026-08-14 18:02 ` Andrey Rachitskiy <pl0h0yp1@gmail.com>
1 sibling, 0 replies; 7+ messages in thread
From: Andrey Rachitskiy @ 2026-08-14 18:02 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Andrey Borodin <x4mmm@yandex-team.ru>; ilia.kashintsev@gmail.com, PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>
пт, 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
^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: BUG #19613: pg_restore: several SEGVs in ReadToc() in pg_backup_archiver.c
2026-08-07 13:37 BUG #19613: pg_restore: several SEGVs in ReadToc() in pg_backup_archiver.c PG Bug reporting form <noreply@postgresql.org>
2026-08-11 07:05 ` Re: BUG #19613: pg_restore: several SEGVs in ReadToc() in pg_backup_archiver.c Andrey Rachitskiy <pl0h0yp1@gmail.com>
2026-08-14 11:58 ` Re: BUG #19613: pg_restore: several SEGVs in ReadToc() in pg_backup_archiver.c Andrey Borodin <x4mmm@yandex-team.ru>
2026-08-14 16:33 ` Re: BUG #19613: pg_restore: several SEGVs in ReadToc() in pg_backup_archiver.c Tom Lane <tgl@sss.pgh.pa.us>
@ 2026-08-16 15:03 ` Andrey Borodin <x4mmm@yandex-team.ru>
1 sibling, 0 replies; 7+ messages in thread
From: Andrey Borodin @ 2026-08-16 15:03 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Andrey Rachitskiy <pl0h0yp1@gmail.com>; ilia.kashintsev@gmail.com, PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>
> 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.
^ permalink raw reply [nested|flat] 7+ messages in thread
end of thread, other threads:[~2026-08-16 15:03 UTC | newest]
Thread overview: 7+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2026-08-07 13:37 BUG #19613: pg_restore: several SEGVs in ReadToc() in pg_backup_archiver.c PG Bug reporting form <noreply@postgresql.org>
2026-08-11 07:05 ` Andrey Rachitskiy <pl0h0yp1@gmail.com>
2026-08-14 11:58 ` Andrey Borodin <x4mmm@yandex-team.ru>
2026-08-14 13:59 ` Andrey Rachitskiy <pl0h0yp1@gmail.com>
2026-08-14 16:33 ` Tom Lane <tgl@sss.pgh.pa.us>
2026-08-14 18:02 ` Andrey Rachitskiy <pl0h0yp1@gmail.com>
2026-08-16 15:03 ` Andrey Borodin <x4mmm@yandex-team.ru>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox