public inbox for pgsql-bugs@postgresql.org  
help / color / mirror / Atom feed
From: Ayush Tiwari <ayushtiwari.slg01@gmail.com>
To: pierre.forstmann@gmail.com
Cc: pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #19476: Segmentation fault in contrib/spi
Date: Wed, 13 May 2026 00:57:47 +0530
Message-ID: <CAJTYsWWh5qsJcLatT5HD9daTEbCnZUZoqaDPGiT=+EyHkEKJ2A@mail.gmail.com> (raw)
In-Reply-To: <1357efa6-dddb-4e60-ba6f-e88d03a4e010@gmail.com>
References: <19476-bd04ea6241345303@postgresql.org>
	<CAJTYsWVuNPbqS2p1gEitRLBHuytqM7OMawuzVH6g4uqGw4RBsQ@mail.gmail.com>
	<1357efa6-dddb-4e60-ba6f-e88d03a4e010@gmail.com>

Hi,


On Wed, 13 May 2026 at 00:22, <pierre.forstmann@gmail.com> wrote:

> Hello,
>
> You have not used the very last version of refint.c which has been updated
> just yesterday:
>
> commit 1ebda7da9a43d3ae3564d08612de9cb27fbaf482
> Author: Nathan Bossart <nathan@postgresql.org>
> Date:   Mon May 11 05:13:48 2026 -0700
>
>     refint: Fix SQL injection and buffer overruns.
>
>     Maliciously crafted key value updates could achieve SQL injection
>     within check_foreign_key().  To fix, ensure new key values are
>     properly quoted and escaped in the internally generated SQL
>     statements.  While at it, avoid potential buffer overruns by
>     replacing the stack buffers for internally generated SQL statements
>     with StringInfo.
>
>     Reported-by: Nikolay Samokhvalov <nik@postgres.ai>
>     Author: Nathan Bossart <nathandbossart@gmail.com>
>     Reviewed-by: Noah Misch <noah@leadboat.com>
>     Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
>     Reviewed-by: Fujii Masao <masao.fujii@gmail.com>
>     Security: CVE-2026-6637
>     Backpatch-through: 14
>
>
You're right, thanks for catching this.  I sent the v1 patches against
master from the day before; commit 260e97733bf (CVE-2026-6637) landed
in between and I had not noticed it.  That commit rewrites the same
cascade-update path to use StringInfo and quote_literal_cstr(), so the
v1 patches do not apply on current master at all.

Importantly, after 260e97733bf the bug is also no longer dependent on
_FORTIFY_SOURCE: the new code calls quote_literal_cstr(nv) directly,
which dereferences nv via strlen() and segfaults on stock builds too.
I reproduced this on plain master built with --enable-cassert.

I have rebased the minimal fix on current master.  It is essentially
the same shape as the snippet you suggested -- emit the NULL keyword
directly when SPI_getvalue() returns NULL, otherwise pass through
quote_literal_cstr() as today.  Attached as v2-0001.

I dropped my earlier 0002 patch.  The CVE fix already addressed the
quoting/escaping concerns that motivated half of it.

Regards,
Ayush


Attachments:

  [application/octet-stream] v2-0001-Fix-refint-cascade-UPDATE-crash-with-NULL-keys.patch (1.8K, 3-v2-0001-Fix-refint-cascade-UPDATE-crash-with-NULL-keys.patch)
  download | inline diff:
From 1f2b319204ba0fae0fbf105e1eb531879c67294b Mon Sep 17 00:00:00 2001
From: Ayush Tiwari <ayushtiwari.slg01@gmail.com>
Date: Tue, 12 May 2026 19:20:08 +0000
Subject: [PATCH v2] Fix refint cascade UPDATE crash with NULL keys

check_foreign_key() builds cascade UPDATE queries using the new key
value retrieved by SPI_getvalue().  After commit 260e97733bf
(CVE-2026-6637) it passes that value through quote_literal_cstr() to
properly escape literals in the generated SQL.

When the new key value is NULL, however, SPI_getvalue() returns a NULL
pointer, which quote_literal_cstr() then dereferences in its strlen()
call, crashing the backend.

Emit the SQL NULL keyword directly when SPI_getvalue() returns NULL.

Reported-by: Nikita Kalinin <n.kalinin@postgrespro.ru>
Discussion: https://postgr.es/m/19476-bd04ea6241345303@postgresql.org
---
 contrib/spi/refint.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/contrib/spi/refint.c b/contrib/spi/refint.c
index c44c87bcd96..5428b511c16 100644
--- a/contrib/spi/refint.c
+++ b/contrib/spi/refint.c
@@ -486,8 +486,17 @@ check_foreign_key(PG_FUNCTION_ARGS)
 						Assert(fn > 0); /* already checked above */
 						nv = SPI_getvalue(newtuple, tupdesc, fn);
 
-						appendStringInfo(&sql, " %s = %s ",
-										 args2[k], quote_literal_cstr(nv));
+						/*
+						 * SPI_getvalue() returns NULL for SQL NULL values.
+						 * Emit the NULL keyword directly rather than passing
+						 * a NULL pointer to quote_literal_cstr(), which would
+						 * dereference it.
+						 */
+						if (nv == NULL)
+							appendStringInfo(&sql, " %s = NULL ", args2[k]);
+						else
+							appendStringInfo(&sql, " %s = %s ",
+											 args2[k], quote_literal_cstr(nv));
 						if (k < nkeys)
 							appendStringInfoString(&sql, ", ");
 					}
-- 
2.43.0



reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: pgsql-bugs@postgresql.org
  Cc: ayushtiwari.slg01@gmail.com, pierre.forstmann@gmail.com, pgsql-bugs@lists.postgresql.org
  Subject: Re: BUG #19476: Segmentation fault in contrib/spi
  In-Reply-To: <CAJTYsWWh5qsJcLatT5HD9daTEbCnZUZoqaDPGiT=+EyHkEKJ2A@mail.gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox