From 2b817dc0a84e84e220e7031f43d76a4e9fb3ece3 Mon Sep 17 00:00:00 2001 From: Joao Detomini Date: Mon, 5 Oct 2026 19:41:10 -0300 Subject: [PATCH] Check RLS policies before evaluating the ON CONFLICT WHERE clause ExecOnConflictUpdate() and ExecOnConflictSelect() evaluated the user-supplied WHERE clause against the existing row before checking the row against the applicable row-level security policies. The documentation says the existing row is checked against the USING expressions first. As a result, a user who cannot see a row because of a SELECT or UPDATE policy could still pass that row's columns to arbitrary expressions in the WHERE clause, such as a function that raises a NOTICE with its argument. Even without a function, a predicate on a hidden column acted as an oracle: an error if it matched, silently no row if it did not. The user only needs to propose a row that conflicts with the hidden row on a unique index. Fix by doing the RLS check first, in both functions. A hidden existing row now raises the RLS error even when the WHERE clause would have been false, as the documentation describes. Add regression tests that use f_leak() in the WHERE clause of both DO UPDATE and DO SELECT against a row hidden by policy. The DO UPDATE path has had this ordering since ON CONFLICT was added in 168d5805e4; ON CONFLICT DO SELECT (88327092ff) copied it. Author: Joao Detomini Backpatch-through: 14 --- src/backend/executor/nodeModifyTable.c | 39 +++++++++++++++-------- src/test/regress/expected/rowsecurity.out | 10 ++++++ src/test/regress/sql/rowsecurity.sql | 9 ++++++ 3 files changed, 44 insertions(+), 14 deletions(-) diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c index 558ceece9b..8130cf4949 100644 --- a/src/backend/executor/nodeModifyTable.c +++ b/src/backend/executor/nodeModifyTable.c @@ -2945,13 +2945,12 @@ ExecOnConflictUpdate(ModifyTableContext *context, econtext->ecxt_innertuple = excludedSlot; econtext->ecxt_outertuple = NULL; - if (!ExecQual(onConflictSetWhere, econtext)) - { - ExecClearTuple(existing); /* see return below */ - InstrCountFiltered1(&mtstate->ps, 1); - return true; /* done with the tuple */ - } - + /* + * The RLS checks below must run before the WHERE clause is evaluated. The + * WHERE clause is user-supplied and can contain leaky functions, which + * must not get to see an existing row that the policies hide from the + * user. + */ if (resultRelInfo->ri_WithCheckOptions != NIL) { /* @@ -2976,6 +2975,13 @@ ExecOnConflictUpdate(ModifyTableContext *context, mtstate->ps.state); } + if (!ExecQual(onConflictSetWhere, econtext)) + { + ExecClearTuple(existing); /* see return below */ + InstrCountFiltered1(&mtstate->ps, 1); + return true; /* done with the tuple */ + } + /* Project the new tuple version */ ExecProject(resultRelInfo->ri_onConflict->oc_ProjInfo); @@ -3095,13 +3101,11 @@ ExecOnConflictSelect(ModifyTableContext *context, econtext->ecxt_innertuple = excludedSlot; econtext->ecxt_outertuple = NULL; - if (!ExecQual(onConflictSelectWhere, econtext)) - { - ExecClearTuple(existing); /* see return below */ - InstrCountFiltered1(&mtstate->ps, 1); - return true; /* done with the tuple */ - } - + /* + * As in ExecOnConflictUpdate(), the RLS checks must run before the WHERE + * clause is evaluated, so that user-supplied expressions never see a row + * that the policies hide from the user. + */ if (resultRelInfo->ri_WithCheckOptions != NIL) { /* @@ -3120,6 +3124,13 @@ ExecOnConflictSelect(ModifyTableContext *context, mtstate->ps.state); } + if (!ExecQual(onConflictSelectWhere, econtext)) + { + ExecClearTuple(existing); /* see return below */ + InstrCountFiltered1(&mtstate->ps, 1); + return true; /* done with the tuple */ + } + /* RETURNING is required for DO SELECT */ Assert(resultRelInfo->ri_projectReturning); diff --git a/src/test/regress/expected/rowsecurity.out b/src/test/regress/expected/rowsecurity.out index 72d7640563..c9a9a4c9d1 100644 --- a/src/test/regress/expected/rowsecurity.out +++ b/src/test/regress/expected/rowsecurity.out @@ -2350,6 +2350,11 @@ INSERT INTO document VALUES (33, 22, 1, 'regress_rls_bob', 'okay science fiction INSERT INTO document VALUES (33, (SELECT cid from category WHERE cname = 'novel'), 1, 'regress_rls_bob', 'Some novel, replaces sci-fi') -- takes UPDATE path ON CONFLICT (did) DO UPDATE SET dtitle = EXCLUDED.dtitle; ERROR: new row violates row-level security policy (USING expression) for table "document" +-- The WHERE clause must not see the existing row before the UPDATE policy has +-- been checked against it (no f_leak() notice expected): +INSERT INTO document VALUES (33, (SELECT cid from category WHERE cname = 'novel'), 1, 'regress_rls_bob', 'Some novel, replaces sci-fi') + ON CONFLICT (did) DO UPDATE SET dtitle = EXCLUDED.dtitle WHERE f_leak(document.dtitle); +ERROR: new row violates row-level security policy (USING expression) for table "document" -- Fine (we UPDATE, since INSERT WCOs and UPDATE security barrier quals + WCOs -- not violated): INSERT INTO document VALUES (2, (SELECT cid from category WHERE cname = 'novel'), 1, 'regress_rls_bob', 'my first novel') @@ -2475,6 +2480,11 @@ INSERT INTO document VALUES (1, (SELECT cid from category WHERE cname = 'novel') INSERT INTO document VALUES (33, (SELECT cid from category WHERE cname = 'science fiction'), 1, 'regress_rls_bob', 'another sci-fi') ON CONFLICT (did) DO SELECT RETURNING did, dauthor, dtitle; ERROR: new row violates row-level security policy for table "document" +-- Likewise, the WHERE clause must not be evaluated against a row that the +-- SELECT policy hides (no f_leak() notice expected) +INSERT INTO document VALUES (33, (SELECT cid from category WHERE cname = 'novel'), 1, 'regress_rls_bob', 'a novel') + ON CONFLICT (did) DO SELECT WHERE f_leak(document.dtitle) RETURNING did, dauthor, dtitle; +ERROR: new row violates row-level security policy (USING expression) for table "document" -- DO SELECT with WHERE and EXCLUDED reference INSERT INTO document VALUES (1, (SELECT cid from category WHERE cname = 'novel'), 1, 'regress_rls_bob', 'another novel') ON CONFLICT (did) DO SELECT WHERE excluded.dlevel = 1 RETURNING did, dauthor, dtitle; diff --git a/src/test/regress/sql/rowsecurity.sql b/src/test/regress/sql/rowsecurity.sql index 2bdb0c70c5..b7ade7d35b 100644 --- a/src/test/regress/sql/rowsecurity.sql +++ b/src/test/regress/sql/rowsecurity.sql @@ -890,6 +890,10 @@ INSERT INTO document VALUES (2, (SELECT cid from category WHERE cname = 'novel') INSERT INTO document VALUES (33, 22, 1, 'regress_rls_bob', 'okay science fiction'); -- preparation for next statement INSERT INTO document VALUES (33, (SELECT cid from category WHERE cname = 'novel'), 1, 'regress_rls_bob', 'Some novel, replaces sci-fi') -- takes UPDATE path ON CONFLICT (did) DO UPDATE SET dtitle = EXCLUDED.dtitle; +-- The WHERE clause must not see the existing row before the UPDATE policy has +-- been checked against it (no f_leak() notice expected): +INSERT INTO document VALUES (33, (SELECT cid from category WHERE cname = 'novel'), 1, 'regress_rls_bob', 'Some novel, replaces sci-fi') + ON CONFLICT (did) DO UPDATE SET dtitle = EXCLUDED.dtitle WHERE f_leak(document.dtitle); -- Fine (we UPDATE, since INSERT WCOs and UPDATE security barrier quals + WCOs -- not violated): INSERT INTO document VALUES (2, (SELECT cid from category WHERE cname = 'novel'), 1, 'regress_rls_bob', 'my first novel') @@ -996,6 +1000,11 @@ INSERT INTO document VALUES (1, (SELECT cid from category WHERE cname = 'novel') INSERT INTO document VALUES (33, (SELECT cid from category WHERE cname = 'science fiction'), 1, 'regress_rls_bob', 'another sci-fi') ON CONFLICT (did) DO SELECT RETURNING did, dauthor, dtitle; +-- Likewise, the WHERE clause must not be evaluated against a row that the +-- SELECT policy hides (no f_leak() notice expected) +INSERT INTO document VALUES (33, (SELECT cid from category WHERE cname = 'novel'), 1, 'regress_rls_bob', 'a novel') + ON CONFLICT (did) DO SELECT WHERE f_leak(document.dtitle) RETURNING did, dauthor, dtitle; + -- DO SELECT with WHERE and EXCLUDED reference INSERT INTO document VALUES (1, (SELECT cid from category WHERE cname = 'novel'), 1, 'regress_rls_bob', 'another novel') ON CONFLICT (did) DO SELECT WHERE excluded.dlevel = 1 RETURNING did, dauthor, dtitle; -- 2.50.1 (Apple Git-155)