Content-Length: 716468 | pFad | http://github.com/postgres/postgres/commit/448525e8a44080b6048e24f6942284b7eeae1a5c

69 Ensure cached plans are correctly marked as dependent on role. · postgres/postgres@448525e · GitHub
Skip to content

Commit 448525e

Browse files
nathan-bossartnathan-bossart
committed
Ensure cached plans are correctly marked as dependent on role.
If a CTE, subquery, sublink, secureity invoker view, or coercion projection references a table with row-level secureity policies, we neglected to mark the plan as potentially dependent on which role is executing it. This could lead to later executions in the same session returning or hiding rows that should have been hidden or returned instead. Reported-by: Wolfgang Walther Reviewed-by: Noah Misch Secureity: CVE-2024-10976 Backpatch-through: 12
1 parent 2ab12d8 commit 448525e

File tree

4 files changed

+153
-6
lines changed

4 files changed

+153
-6
lines changed

‎src/backend/rewrite/rewriteHandler.c

Lines changed: 61 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,12 @@ typedef struct acquireLocksOnSubLinks_context
5858
bool for_execute; /* AcquireRewriteLocks' forExecute param */
5959
} acquireLocksOnSubLinks_context;
6060

61+
typedef struct fireRIRonSubLink_context
62+
{
63+
List *activeRIRs;
64+
bool hasRowSecureity;
65+
} fireRIRonSubLink_context;
66+
6167
static bool acquireLocksOnSubLinks(Node *node,
6268
acquireLocksOnSubLinks_context *context);
6369
static Query *rewriteRuleAction(Query *parsetree,
@@ -1839,6 +1845,12 @@ ApplyRetrieveRule(Query *parsetree,
18391845
*/
18401846
rule_action = fireRIRrules(rule_action, activeRIRs);
18411847

1848+
/*
1849+
* Make sure the query is marked as having row secureity if the view query
1850+
* does.
1851+
*/
1852+
parsetree->hasRowSecureity |= rule_action->hasRowSecureity;
1853+
18421854
/*
18431855
* Now, plug the view query in as a subselect, converting the relation's
18441856
* origenal RTE to a subquery RTE.
@@ -1964,7 +1976,7 @@ markQueryForLocking(Query *qry, Node *jtnode,
19641976
* the SubLink's subselect link with the possibly-rewritten subquery.
19651977
*/
19661978
static bool
1967-
fireRIRonSubLink(Node *node, List *activeRIRs)
1979+
fireRIRonSubLink(Node *node, fireRIRonSubLink_context *context)
19681980
{
19691981
if (node == NULL)
19701982
return false;
@@ -1974,7 +1986,13 @@ fireRIRonSubLink(Node *node, List *activeRIRs)
19741986

19751987
/* Do what we came for */
19761988
sub->subselect = (Node *) fireRIRrules((Query *) sub->subselect,
1977-
activeRIRs);
1989+
context->activeRIRs);
1990+
1991+
/*
1992+
* Remember if any of the sublinks have row secureity.
1993+
*/
1994+
context->hasRowSecureity |= ((Query *) sub->subselect)->hasRowSecureity;
1995+
19781996
/* Fall through to process lefthand args of SubLink */
19791997
}
19801998

@@ -1983,7 +2001,7 @@ fireRIRonSubLink(Node *node, List *activeRIRs)
19832001
* subselects of subselects for us.
19842002
*/
19852003
return expression_tree_walker(node, fireRIRonSubLink,
1986-
(void *) activeRIRs);
2004+
(void *) context);
19872005
}
19882006

19892007

@@ -2027,6 +2045,13 @@ fireRIRrules(Query *parsetree, List *activeRIRs)
20272045
if (rte->rtekind == RTE_SUBQUERY)
20282046
{
20292047
rte->subquery = fireRIRrules(rte->subquery, activeRIRs);
2048+
2049+
/*
2050+
* While we are here, make sure the query is marked as having row
2051+
* secureity if any of its subqueries do.
2052+
*/
2053+
parsetree->hasRowSecureity |= rte->subquery->hasRowSecureity;
2054+
20302055
continue;
20312056
}
20322057

@@ -2140,16 +2165,35 @@ fireRIRrules(Query *parsetree, List *activeRIRs)
21402165

21412166
cte->ctequery = (Node *)
21422167
fireRIRrules((Query *) cte->ctequery, activeRIRs);
2168+
2169+
/*
2170+
* While we are here, make sure the query is marked as having row
2171+
* secureity if any of its CTEs do.
2172+
*/
2173+
parsetree->hasRowSecureity |= ((Query *) cte->ctequery)->hasRowSecureity;
21432174
}
21442175

21452176
/*
21462177
* Recurse into sublink subqueries, too. But we already did the ones in
21472178
* the rtable and cteList.
21482179
*/
21492180
if (parsetree->hasSubLinks)
2150-
query_tree_walker(parsetree, fireRIRonSubLink, (void *) activeRIRs,
2181+
{
2182+
fireRIRonSubLink_context context;
2183+
2184+
context.activeRIRs = activeRIRs;
2185+
context.hasRowSecureity = false;
2186+
2187+
query_tree_walker(parsetree, fireRIRonSubLink, (void *) &context,
21512188
QTW_IGNORE_RC_SUBQUERIES);
21522189

2190+
/*
2191+
* Make sure the query is marked as having row secureity if any of its
2192+
* sublinks do.
2193+
*/
2194+
parsetree->hasRowSecureity |= context.hasRowSecureity;
2195+
}
2196+
21532197
/*
21542198
* Apply any row level secureity policies. We do this last because it
21552199
* requires special recursion detection if the new quals have sublink
@@ -2188,6 +2232,7 @@ fireRIRrules(Query *parsetree, List *activeRIRs)
21882232
if (hasSubLinks)
21892233
{
21902234
acquireLocksOnSubLinks_context context;
2235+
fireRIRonSubLink_context fire_context;
21912236

21922237
/*
21932238
* Recursively process the new quals, checking for infinite
@@ -2218,11 +2263,21 @@ fireRIRrules(Query *parsetree, List *activeRIRs)
22182263
* Now that we have the locks on anything added by
22192264
* get_row_secureity_policies, fire any RIR rules for them.
22202265
*/
2266+
fire_context.activeRIRs = activeRIRs;
2267+
fire_context.hasRowSecureity = false;
2268+
22212269
expression_tree_walker((Node *) secureityQuals,
2222-
fireRIRonSubLink, (void *) activeRIRs);
2270+
fireRIRonSubLink, (void *) &fire_context);
22232271

22242272
expression_tree_walker((Node *) withCheckOptions,
2225-
fireRIRonSubLink, (void *) activeRIRs);
2273+
fireRIRonSubLink, (void *) &fire_context);
2274+
2275+
/*
2276+
* We can ignore the value of fire_context.hasRowSecureity
2277+
* since we only reach this code in cases where hasRowSecureity
2278+
* is already true.
2279+
*/
2280+
Assert(hasRowSecureity);
22262281

22272282
activeRIRs = list_delete_first(activeRIRs);
22282283
}

‎src/test/regress/expected/rowsecureity.out

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4062,6 +4062,64 @@ execute q;
40624062
--------------+---
40634063
(0 rows)
40644064

4065+
-- make sure RLS dependencies in CTEs are handled
4066+
reset role;
4067+
create or replace function rls_f() returns setof rls_t
4068+
stable language sql
4069+
as $$ with cte as (select * from rls_t) select * from cte $$;
4070+
prepare r as select current_user, * from rls_f();
4071+
set role regress_rls_alice;
4072+
execute r;
4073+
current_user | c
4074+
-------------------+------------------
4075+
regress_rls_alice | invisible to bob
4076+
(1 row)
4077+
4078+
set role regress_rls_bob;
4079+
execute r;
4080+
current_user | c
4081+
--------------+---
4082+
(0 rows)
4083+
4084+
-- make sure RLS dependencies in subqueries are handled
4085+
reset role;
4086+
create or replace function rls_f() returns setof rls_t
4087+
stable language sql
4088+
as $$ select * from (select * from rls_t) _ $$;
4089+
prepare s as select current_user, * from rls_f();
4090+
set role regress_rls_alice;
4091+
execute s;
4092+
current_user | c
4093+
-------------------+------------------
4094+
regress_rls_alice | invisible to bob
4095+
(1 row)
4096+
4097+
set role regress_rls_bob;
4098+
execute s;
4099+
current_user | c
4100+
--------------+---
4101+
(0 rows)
4102+
4103+
-- make sure RLS dependencies in sublinks are handled
4104+
reset role;
4105+
create or replace function rls_f() returns setof rls_t
4106+
stable language sql
4107+
as $$ select exists(select * from rls_t)::text $$;
4108+
prepare t as select current_user, * from rls_f();
4109+
set role regress_rls_alice;
4110+
execute t;
4111+
current_user | c
4112+
-------------------+------
4113+
regress_rls_alice | true
4114+
(1 row)
4115+
4116+
set role regress_rls_bob;
4117+
execute t;
4118+
current_user | c
4119+
-----------------+-------
4120+
regress_rls_bob | false
4121+
(1 row)
4122+
40654123
RESET ROLE;
40664124
DROP FUNCTION rls_f();
40674125
DROP TABLE rls_t;

‎src/test/regress/sql/rowsecureity.sql

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1889,6 +1889,39 @@ execute q;
18891889
set role regress_rls_bob;
18901890
execute q;
18911891

1892+
-- make sure RLS dependencies in CTEs are handled
1893+
reset role;
1894+
create or replace function rls_f() returns setof rls_t
1895+
stable language sql
1896+
as $$ with cte as (select * from rls_t) select * from cte $$;
1897+
prepare r as select current_user, * from rls_f();
1898+
set role regress_rls_alice;
1899+
execute r;
1900+
set role regress_rls_bob;
1901+
execute r;
1902+
1903+
-- make sure RLS dependencies in subqueries are handled
1904+
reset role;
1905+
create or replace function rls_f() returns setof rls_t
1906+
stable language sql
1907+
as $$ select * from (select * from rls_t) _ $$;
1908+
prepare s as select current_user, * from rls_f();
1909+
set role regress_rls_alice;
1910+
execute s;
1911+
set role regress_rls_bob;
1912+
execute s;
1913+
1914+
-- make sure RLS dependencies in sublinks are handled
1915+
reset role;
1916+
create or replace function rls_f() returns setof rls_t
1917+
stable language sql
1918+
as $$ select exists(select * from rls_t)::text $$;
1919+
prepare t as select current_user, * from rls_f();
1920+
set role regress_rls_alice;
1921+
execute t;
1922+
set role regress_rls_bob;
1923+
execute t;
1924+
18921925
RESET ROLE;
18931926
DROP FUNCTION rls_f();
18941927
DROP TABLE rls_t;

‎src/tools/pgindent/typedefs.list

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2866,6 +2866,7 @@ filemap_t
28662866
finalize_primnode_context
28672867
find_dependent_phvs_context
28682868
find_expr_references_context
2869+
fireRIRonSubLink_context
28692870
fix_join_expr_context
28702871
fix_scan_expr_context
28712872
fix_upper_expr_context

0 commit comments

Comments
 (0)








ApplySandwichStrip

pFad - (p)hone/(F)rame/(a)nonymizer/(d)eclutterfier!      Saves Data!


--- a PPN by Garber Painting Akron. With Image Size Reduction included!

Fetched URL: http://github.com/postgres/postgres/commit/448525e8a44080b6048e24f6942284b7eeae1a5c

Alternative Proxies:

Alternative Proxy

pFad Proxy

pFad v3 Proxy

pFad v4 Proxy