public inbox for pgsql-bugs@postgresql.org  

help / color / mirror / Atom feed

From: Nathan Bossart <nathandbossart@gmail.com>
To: Ayush Tiwari <ayushtiwari.slg01@gmail.com>
Cc: pierre.forstmann@gmail.com
Cc: pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #19476: Segmentation fault in contrib/spi
Date: Thu, 14 May 2026 11:01:55 -0500
Message-ID: <agXx84rrzKWY9aAE@nathan> (raw)
In-Reply-To: <CAJTYsWWh5qsJcLatT5HD9daTEbCnZUZoqaDPGiT=+EyHkEKJ2A@mail.gmail.com>
References: <19476-bd04ea6241345303@postgresql.org>
	<CAJTYsWVuNPbqS2p1gEitRLBHuytqM7OMawuzVH6g4uqGw4RBsQ@mail.gmail.com>
	<1357efa6-dddb-4e60-ba6f-e88d03a4e010@gmail.com>
	<CAJTYsWWh5qsJcLatT5HD9daTEbCnZUZoqaDPGiT=+EyHkEKJ2A@mail.gmail.com>

On Wed, May 13, 2026 at 12:57:47AM +0530, Ayush Tiwari wrote:
> 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.

I'm confused why you dropped 0002.  Reusing the new key values for
subsequent updates seems like a bug worth fixing.  However, note that the
parameter symbol approach doesn't work well for cases like this:

    CREATE EXTENSION refint;

    CREATE TABLE p (a int);
    CREATE TABLE f (a xid);
    CREATE TRIGGER t
        AFTER DELETE OR UPDATE ON p
        FOR EACH ROW EXECUTE PROCEDURE
        check_foreign_key(1, 'c', 'a', 'f', 'a');

    INSERT INTO p VALUES (1);
    UPDATE p SET a = 2;

With a rebased version of 0002 applied, the UPDATE statement fails like
this:

    ERROR:  column "a" is of type xid but expression is of type integer
    LINE 1: update f set  a = $2  where a = $1
                              ^
    HINT:  You will need to rewrite or cast the expression.
    QUERY:  update f set  a = $2  where a = $1

Presumably the problem is that we're using the wrong argument type for the
foreign key.  I'm not sure this is trivial to fix; it seems like we'd need
to provide that information in CREATE TRIGGER or look up the foreign key
type within the trigger function itself.  Perhaps the best we can do is to
avoid caching a plan in this case.

Regarding 0001, note that the refint docs state the following:

    Note that the primary/unique key columns should be marked NOT NULL and
    should have a unique index.

So maybe we could alternatively teach check_foreign_key() to either ERROR
or do nothing instead.  On the other hand, given this case seemed to
accidentally work before the CVE fix, it's arguably worth fixing.

-- 
nathan

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: nathandbossart@gmail.com, 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: <agXx84rrzKWY9aAE@nathan>

* 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