Re: BUG #19762: ManualBug1 tcn: NULL pointer dereference in triggered_change_notification() via strcpy_quoted

On 10/15/26 17:02, PG Bug reporting form wrote:
> The following bug has been logged on the website:
>
> Bug reference: 19762
> Logged by: Yuelin Wang
> Email address: 1217816127(at)qq(dot)com
> PostgreSQL version: 19rc1
> Operating system: Linux (Ubuntu 24.04, x86_64)
> Description:
>
> ### What happens?
>
> In `contrib/tcn/tcn.c` at line 165, the function
> `triggered_change_notification()` calls `SPI_getvalue(trigtuple, tupdesc,
> colno)` and passes the return value directly to `strcpy_quoted()` without
> checking whether it is NULL:
>
> ```c
> /* contrib/tcn/tcn.c:165 */
> strcpy_quoted(&payload, SPI_getvalue(trigtuple, tupdesc, colno), '\'');
> ```
>
> `SPI_getvalue()` returns NULL when the requested column value is SQL NULL
> (it sets `isnull = true` and returns NULL from `heap_getattr`).
> The function `strcpy_quoted()` defined at line 36 immediately dereferences
> the pointer at line 39:
>
> ```c
> static void
> strcpy_quoted(StringInfo r, const char *s, const char q)
> {
> appendStringInfoCharMacro(r, q);
> while (*s) /* line 39: unconditional dereference */
> ...
> }
> ```
>
> The violated invariant is: any caller of `strcpy_quoted` must guarantee the
> string pointer `s` is non-NULL before passing it.
> The Entry61 fix in `refint.c` showed the identical invariant being violated
> by `quote_literal_cstr(nv)` where `nv` could be NULL.
> Here the same class of bug appears: a nullable value returned by
> `SPI_getvalue` is passed to a string-consuming function that dereferences
> the pointer unconditionally.
> The column `colno` comes from `index->indkey.values[i]` for the primary key
> index.
> Under normal DDL, primary key columns are NOT NULL, which prevents
> `SPI_getvalue` from returning NULL.
> However, the code contains no guard against NULL and will crash if
> `SPI_getvalue` returns NULL for any reason (for example when a superuser
> bypasses the NOT NULL constraint by directly updating
> `pg_index.indisprimary` on an index whose key columns lack a NOT NULL
> constraint, or if a future code path makes the invariant reachable through
> other means).
> The correct fix mirrors Entry61: replace the bare `SPI_getvalue(...)`
> argument with a ternary expression that substitutes the unquoted SQL keyword
> `NULL` when the value is NULL, exactly as `dblink.c` already does at lines
> 2197-2203.
>
> ### To Reproduce
>
> Start a fresh PostgreSQL ASAN instance and run:
>
> ```sql
> CREATE EXTENSION tcn;
>
> CREATE TABLE tcn_test (id int, data text);
>
> CREATE UNIQUE INDEX tcn_test_uniq ON tcn_test (id);
>
> SET allow_system_table_mods = on;
>
> UPDATE pg_index SET indisprimary = true
> WHERE indexrelid = 'tcn_test_uniq'::regclass;
>

I don't think this counts as a bug.

It requires superuser privileges to update catalogs to break a primary
key - effectively introducing data corruption. Pretty sure there are
countless other places relying on PK columns being NOT NULL.

A superuser can introduce all kinds of corrupted data and cause far
worse stuff. If you could trigger this without requiring superuser
privileges, that'd be a different story, ofc.

regards

--
Tomas Vondra