public inbox for pgsql-bugs@postgresql.org
help / color / mirror / Atom feed[PATCH] Replace debug-only Asserts with runtime checks in logical replication apply worker
2+ messages / 2 participants
[nested] [flat]
* [PATCH] Replace debug-only Asserts with runtime checks in logical replication apply worker
@ 2026-05-16 22:30 Varik Matevosyan <varikmatevosyan@gmail.com>
2026-05-17 01:40 ` Re: [PATCH] Replace debug-only Asserts with runtime checks in logical replication apply worker Noah Misch <noah@leadboat.com>
0 siblings, 1 reply; 2+ messages in thread
From: Varik Matevosyan @ 2026-05-16 22:30 UTC (permalink / raw)
To: pgsql-bugs@lists.postgresql.org; +Cc: Noah Misch <noah@leadboat.com>
The attached patch replaces three debug-only Asserts with runtime
ereport(ERROR, ERRCODE_PROTOCOL_VIOLATION) checks in the logical
replication apply worker (worker.c). These guard against a mismatch
between the column count in the RELATION message and the count in a
subsequent INSERT/UPDATE/DELETE tuple message.
A publisher can send a RELATION claiming N columns and
an INSERT claiming M < N columns, causing the subscriber
to index past the end of the tuple's colvalues[]/colstatus[] arrays.
I believe this is more of a correctness fix than a security issue as
the attacker needs replication privileges, and in my testing I was not
able to trigger a SIGSEGV, the OOB read landed on heap bytes that
happened to not cause a crash.
P.S: After a security review from Noah, I'm reporting this as a bug.
Thanks,
Varik
Attachments:
[application/octet-stream] 0001-Replace-debug-only-Asserts-with-runtime-checks-in-lo.patch (3.1K, 2-0001-Replace-debug-only-Asserts-with-runtime-checks-in-lo.patch)
download | inline diff:
From f7695baed40e5bb206d7e42910eaf0348b7fdbac Mon Sep 17 00:00:00 2001
From: Varik Matevosyan <varikmatevosyan@gmail.com>
Date: Mon, 4 May 2026 14:33:09 +0000
Subject: [PATCH] Replace debug-only Asserts with runtime checks in logical
replication apply worker
Three locations in worker.c use Assert() to guard against a mismatch
between the number of columns advertised in the RELATION message and
the number actually received in the subsequent INSERT/UPDATE tuple
message. Since these values originate from the publisher, the check
must survive into production builds.
A malicious or buggy publisher can send a RELATION claiming N columns
and an INSERT claiming M < N columns. The subscriber's apply worker
indexes into colvalues[]/colstatus[] using column indices from the
RELATION message's attribute map, causing a heap out-of-bounds read
when the tuple's column array is smaller than expected.
Replace the Asserts with ereport(ERROR, ERRCODE_PROTOCOL_VIOLATION)
that produces a clear diagnostic and cleanly aborts the transaction.
---
src/backend/replication/logical/worker.c | 23 +++++++++++++++++++----
1 file changed, 19 insertions(+), 4 deletions(-)
diff --git a/src/backend/replication/logical/worker.c b/src/backend/replication/logical/worker.c
index dd6fc38a41e..a3f2406ed83 100644
--- a/src/backend/replication/logical/worker.c
+++ b/src/backend/replication/logical/worker.c
@@ -1038,9 +1038,15 @@ slot_store_data(TupleTableSlot *slot, LogicalRepRelMapEntry *rel,
if (!att->attisdropped && remoteattnum >= 0)
{
- StringInfo colvalue = &tupleData->colvalues[remoteattnum];
+ StringInfo colvalue;
+
+ if (remoteattnum >= tupleData->ncols)
+ ereport(ERROR,
+ (errcode(ERRCODE_PROTOCOL_VIOLATION),
+ errmsg("logical replication column %d not found in tuple: only %d column(s) received",
+ remoteattnum + 1, tupleData->ncols)));
- Assert(remoteattnum < tupleData->ncols);
+ colvalue = &tupleData->colvalues[remoteattnum];
/* Set attnum for error callback */
apply_error_callback_arg.remote_attnum = remoteattnum;
@@ -1151,7 +1157,11 @@ slot_modify_data(TupleTableSlot *slot, TupleTableSlot *srcslot,
if (remoteattnum < 0)
continue;
- Assert(remoteattnum < tupleData->ncols);
+ if (remoteattnum >= tupleData->ncols)
+ ereport(ERROR,
+ (errcode(ERRCODE_PROTOCOL_VIOLATION),
+ errmsg("logical replication column %d not found in tuple: only %d column(s) received",
+ remoteattnum + 1, tupleData->ncols)));
if (tupleData->colstatus[remoteattnum] != LOGICALREP_COLUMN_UNCHANGED)
{
@@ -2870,7 +2880,12 @@ apply_handle_update(StringInfo s)
if (!att->attisdropped && remoteattnum >= 0)
{
- Assert(remoteattnum < newtup.ncols);
+ if (remoteattnum >= newtup.ncols)
+ ereport(ERROR,
+ (errcode(ERRCODE_PROTOCOL_VIOLATION),
+ errmsg("logical replication column %d not found in tuple: only %d column(s) received",
+ remoteattnum + 1, newtup.ncols)));
+
if (newtup.colstatus[remoteattnum] != LOGICALREP_COLUMN_UNCHANGED)
target_perminfo->updatedCols =
bms_add_member(target_perminfo->updatedCols,
--
2.43.0
^ permalink raw reply [nested|flat] 2+ messages in thread
* Re: [PATCH] Replace debug-only Asserts with runtime checks in logical replication apply worker
2026-05-16 22:30 [PATCH] Replace debug-only Asserts with runtime checks in logical replication apply worker Varik Matevosyan <varikmatevosyan@gmail.com>
@ 2026-05-17 01:40 ` Noah Misch <noah@leadboat.com>
0 siblings, 0 replies; 2+ messages in thread
From: Noah Misch @ 2026-05-17 01:40 UTC (permalink / raw)
To: Varik Matevosyan <varikmatevosyan@gmail.com>; +Cc: pgsql-bugs@lists.postgresql.org
On Sun, May 17, 2026 at 02:30:00AM +0400, Varik Matevosyan wrote:
> The attached patch replaces three debug-only Asserts with runtime
> ereport(ERROR, ERRCODE_PROTOCOL_VIOLATION) checks in the logical
> replication apply worker (worker.c). These guard against a mismatch
> between the column count in the RELATION message and the count in a
> subsequent INSERT/UPDATE/DELETE tuple message.
>
> A publisher can send a RELATION claiming N columns and
> an INSERT claiming M < N columns, causing the subscriber
> to index past the end of the tuple's colvalues[]/colstatus[] arrays.
>
> I believe this is more of a correctness fix than a security issue as
> the attacker needs replication privileges, and in my testing I was not
> able to trigger a SIGSEGV, the OOB read landed on heap bytes that
> happened to not cause a crash.
>
> P.S: After a security review from Noah, I'm reporting this as a bug.
Pushed (bf7d19b). Thank you.
^ permalink raw reply [nested|flat] 2+ messages in thread
end of thread, other threads:[~2026-05-17 01:40 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2026-05-16 22:30 [PATCH] Replace debug-only Asserts with runtime checks in logical replication apply worker Varik Matevosyan <varikmatevosyan@gmail.com>
2026-05-17 01:40 ` Noah Misch <noah@leadboat.com>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox