Re: BUG #19476: Segmentation fault in contrib/spi

Hello,

You have not used the very last version of refint.c which has been updated just yesterday:

commit 1ebda7da9a43d3ae3564d08612de9cb27fbaf482
Author: Nathan Bossart <nathan(at)postgresql(dot)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(at)postgres(dot)ai>
Author: Nathan Bossart <nathandbossart(at)gmail(dot)com>
Reviewed-by: Noah Misch <noah(at)leadboat(dot)com>
Reviewed-by: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Reviewed-by: Fujii Masao <masao(dot)fujii(at)gmail(dot)com>
Security: CVE-2026-6637
Backpatch-through: 14

I think for patch v1-0001 you need to adapt the code with something like:

/* SPI_getvalue() returns NULL for SQL NULL values, so
* emit the NULL keyword rather than passing a NULL
* pointer to quote_litteral_cstr, which is undefined
* behavior.
*/
if (nv == NULL)
appendStringInfo(&sql, " %s = NULL %s",
args2[k], "");
else
appendStringInfo(&sql, " %s = %s ",
args2[k], quote_literal_cstr(nv));

Regards
Pierre

On 12/05/2026 20:04, Ayush Tiwari <ayushtiwari(dot)slg01(at)gmail(dot)com> wrote:
> Hi,
>
> On Tue, 12 May 2026 at 21:14, PG Bug reporting form
> <noreply(at)postgresql(dot)org <mailto:noreply(at)postgresql(dot)org>> wrote:
>
>
> CREATE EXTENSION refint;
> CREATE TABLE c (id int4);
> CREATE UNIQUE INDEX ci ON c(id);
> CREATE TABLE b (refb int4);
> CREATE INDEX bi ON b(refb);
>
> CREATE TRIGGER at AFTER DELETE OR UPDATE ON c FOR EACH ROW
> EXECUTE FUNCTION check_foreign_key (1, 'cascade', 'id', 'b', 'refb');
>
> CREATE TRIGGER bt AFTER INSERT OR UPDATE ON b FOR EACH ROW
> EXECUTE FUNCTION check_primary_key ('refb', 'c', 'id');
>
> INSERT INTO c VALUES (10);
> INSERT INTO b VALUES (10);
> UPDATE c SET id = NULL WHERE id = 10;
>
> Backtrace:
> #0  __strlen_avx2 () at ../sysdeps/x86_64/multiarch/strlen-avx2.S:76
> #1  0x0000563a92ca096e in quote_literal_cstr (rawstr=0x0) at quote.c:109
> #2  0x00007fa4a7cce5f8 in check_foreign_key (fcinfo=<optimized out>) at
> refint.c:489
> #3  0x0000563a929c1d52 in ExecCallTriggerFunc
> (trigdata=trigdata(at)entry=0x7fff8cae1100,
>     tgindx=tgindx(at)entry=0, finfo=finfo(at)entry=0x563aa5ae4438,
> instr=instr(at)entry=0x0,
>     per_tuple_context=per_tuple_context(at)entry=0x563aa5adb9c0) at
> trigger.c:2369
> #4  0x0000563a929c4110 in AfterTriggerExecute (estate=<optimized out>,
> event=0x563aa5aedbb0,
>     relInfo=0x563aa5ae4068, src_relInfo=<optimized out>,
> dst_relInfo=<optimized out>,
>     trigdesc=0x563aa5ae4278, finfo=0x563aa5ae4438, instr=0x0,
>     per_tuple_context=<optimized out>, trig_tuple_slot1=0x0,
> trig_tuple_slot2=0x0)
>     at trigger.c:4559
> #5  afterTriggerInvokeEvents (events=events(at)entry=0x563aa5a75050,
> firing_id=1,
>     estate=estate(at)entry=0x563aa5ae3b20,
> delete_ok=delete_ok(at)entry=false) at
> trigger.c:4800
> #6  0x0000563a929c8e88 in AfterTriggerEndQuery
> (estate=estate(at)entry=0x563aa5ae3b20)
>     at trigger.c:5182
> #7  0x0000563a929ed504 in standard_ExecutorFinish
> (queryDesc=0x563aa5996e20)
> at execMain.c:443
> #8  0x0000563a92bd96f8 in ProcessQuery (plan=0x563aa5af4168,
>     sourceText=0x563aa59df7e0 "UPDATE c SET id = NULL WHERE id = 10;",
> params=0x0,
>     queryEnv=0x0, dest=0x563aa5aeae28, qc=0x7fff8cae1460) at
> pquery.c:194
> #9  0x0000563a92bda41e in PortalRunMulti
> (portal=portal(at)entry=0x563aa5a60510,
>     isTopLevel=isTopLevel(at)entry=true,
> setHoldSnapshot=setHoldSnapshot(at)entry=false,
> --Type <RET> for more, q to quit, c to continue without paging--c
>     dest=dest(at)entry=0x563aa5aeae28,
> altdest=altdest(at)entry=0x563aa5aeae28,
>     qc=qc(at)entry=0x7fff8cae1460) at pquery.c:1272
> #10 0x0000563a92bda7f7 in PortalRun (portal=portal(at)entry=0x563aa5a60510,
>     count=count(at)entry=9223372036854775807,
> isTopLevel=isTopLevel(at)entry=true,
>     dest=dest(at)entry=0x563aa5aeae28,
> altdest=altdest(at)entry=0x563aa5aeae28,
>     qc=qc(at)entry=0x7fff8cae1460) at pquery.c:788
> #11 0x0000563a92bd63fa in exec_simple_query (
>     query_string=0x563aa59df7e0 "UPDATE c SET id = NULL WHERE id =
> 10;") at
> postgres.c:1274
> #12 0x0000563a92bd7fc5 in PostgresMain (dbname=<optimized out>,
> username=<optimized out>)
>     at postgres.c:4770
> #13 0x0000563a92bd23bd in BackendMain (startup_data=<optimized out>,
>     startup_data_len=<optimized out>) at backend_startup.c:124
> #14 0x0000563a92b24932 in postmaster_child_launch (child_type=<optimized
> out>, child_slot=1,
>     startup_data=startup_data(at)entry=0x7fff8cae1900,
>     startup_data_len=startup_data_len(at)entry=24,
> client_sock=client_sock(at)entry=0x7fff8cae1920)
>     at launch_backend.c:290
> #15 0x0000563a92b283d2 in BackendStartup (client_sock=0x7fff8cae1920) at
> postmaster.c:3569
> #16 ServerLoop () at postmaster.c:1703
> #17 0x0000563a92b29ea6 in PostmasterMain (argc=argc(at)entry=3,
> argv=argv(at)entry=0x563aa5985bf0)
>     at postmaster.c:1401
> #18 0x0000563a92800a5a in main (argc=3, argv=0x563aa5985bf0) at
> main.c:227
>
>
> Thanks for the report and the clear backtrace.
>
> I had a look at this and I think the immediate crash comes from
> check_foreign_key()'s cascade-update branch in contrib/spi/refint.c,
> which calls SPI_getvalue() for the new key value and then passes the
> result straight into snprintf("%s", ...):
>
>     nv = SPI_getvalue(newtuple, tupdesc, fn);
>     ...
>     snprintf(sql + strlen(sql), sizeof(sql) - strlen(sql),
>              " %s = %s%s%s %s ",
>              args2[k], (is_char_type > 0) ? "'" : "",
>              nv, (is_char_type > 0) ? "'" : "", (k < nkeys) ? ", " : "");
>
> For an UPDATE that sets the referenced key to NULL, SPI_getvalue()
> returns NULL, so nv is NULL.  On stock glibc/Postgres builds snprintf
> substitutes "(null)" and SQL parses that as the NULL keyword, so the
> cascade silently does the right thing.  On builds with _FORTIFY_SOURCE
> glibc's strlen check dereferences the NULL pointer instead, which
> matches the backtrace you posted.
>
> I am attaching a two-patch series that addresses this.
>
> 0001 is the minimal fix: emit the SQL NULL keyword when SPI_getvalue()
> returns NULL, instead of passing a NULL pointer to snprintf.  This is
> small and targeted, and is sufficient on its own to fix the reported
> crash.  I did not add a regression test for 0001 because the bug only
> manifests on hardened libc builds; on stock builds the bad snprintf
> happens to produce a result that the SQL parser accepts.
>
> 0002 is a follow-up that I think makes sense but is not strictly
> required for this bug.  While looking at the cascade-update path I
> noticed two pre-existing issues in the same code:
>
>  - The new key values were embedded into the prepared cascade UPDATE
>    query text as literals.  Combined with plan caching (the cache key
>    only gained the operation type in 8cfbdf8f4df), the first set of
>    new values gets reused for every subsequent cascade UPDATE.
>
>  - Char-like new values were wrapped in single quotes without escaping,
>    so a value containing a single quote produced malformed SQL.
>
> 0002 switches that branch to use SPI parameters for the new key values
> instead of embedding them into the SQL text.  That naturally handles
> NULLs via the SPI nulls array, lets a single cached plan handle later
> UPDATEs with different keys, and avoids the quoting issue entirely.
> It also adds a refint regression test that runs two cascade UPDATEs in
> a row (one to NULL, one to a non-NULL value) which catches the cached-
> plan reuse.
>
> I'm happy with just 0001 if reviewers prefer to keep the fix minimal,
> given that refint is intended mainly as a documentation example
> (8cfbdf8f4df explicitly mentioned not back-patching changes here?).
>
> Regards,
> Ayush
>