This is an automated email from the ASF dual-hosted git repository. tuhaihe pushed a commit to branch REL_2_STABLE in repository https://gitbox.apache.org/repos/asf/cloudberry.git
commit 251f21f3497c47cb10751aa16f3abaf7f518d081 Author: Nathan Bossart <[email protected]> AuthorDate: Mon Nov 11 09:00:00 2024 -0600 Ensure cached plans are correctly marked as dependent on role. If a CTE, subquery, sublink, security invoker view, or coercion projection references a table with row-level security 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 Security: CVE-2024-10976 Backpatch-through: 12 --- src/backend/executor/functions.c | 6 +++ src/backend/rewrite/rewriteHandler.c | 67 +++++++++++++++++++++++--- src/test/regress/expected/rowsecurity.out | 78 +++++++++++++++++++++++++++++++ src/test/regress/sql/rowsecurity.sql | 44 +++++++++++++++++ src/tools/pgindent/typedefs.list | 1 + 5 files changed, 190 insertions(+), 6 deletions(-) diff --git a/src/backend/executor/functions.c b/src/backend/executor/functions.c index 41da04f652b..f1b287d8ff4 100644 --- a/src/backend/executor/functions.c +++ b/src/backend/executor/functions.c @@ -2120,6 +2120,12 @@ tlist_coercion_finished: rtr->rtindex = 1; newquery->jointree = makeFromExpr(list_make1(rtr), NULL); + /* + * Make sure the new query is marked as having row security if the + * original one does. + */ + newquery->hasRowSecurity = parse->hasRowSecurity; + /* Replace original query in the correct element of the query list */ lfirst(parse_cell) = newquery; } diff --git a/src/backend/rewrite/rewriteHandler.c b/src/backend/rewrite/rewriteHandler.c index 2938edf1c6a..9670eb5d34a 100644 --- a/src/backend/rewrite/rewriteHandler.c +++ b/src/backend/rewrite/rewriteHandler.c @@ -59,6 +59,12 @@ typedef struct acquireLocksOnSubLinks_context bool for_execute; /* AcquireRewriteLocks' forExecute param */ } acquireLocksOnSubLinks_context; +typedef struct fireRIRonSubLink_context +{ + List *activeRIRs; + bool hasRowSecurity; +} fireRIRonSubLink_context; + static bool acquireLocksOnSubLinks(Node *node, acquireLocksOnSubLinks_context *context); static Query *rewriteRuleAction(Query *parsetree, @@ -1910,6 +1916,12 @@ ApplyRetrieveRule(Query *parsetree, */ rule_action = fireRIRrules(rule_action, activeRIRs); + /* + * Make sure the query is marked as having row security if the view query + * does. + */ + parsetree->hasRowSecurity |= rule_action->hasRowSecurity; + /* * Now, plug the view query in as a subselect, converting the relation's * original RTE to a subquery RTE. @@ -2021,7 +2033,7 @@ markQueryForLocking(Query *qry, Node *jtnode, * the SubLink's subselect link with the possibly-rewritten subquery. */ static bool -fireRIRonSubLink(Node *node, List *activeRIRs) +fireRIRonSubLink(Node *node, fireRIRonSubLink_context *context) { if (node == NULL) return false; @@ -2031,7 +2043,13 @@ fireRIRonSubLink(Node *node, List *activeRIRs) /* Do what we came for */ sub->subselect = (Node *) fireRIRrules((Query *) sub->subselect, - activeRIRs); + context->activeRIRs); + + /* + * Remember if any of the sublinks have row security. + */ + context->hasRowSecurity |= ((Query *) sub->subselect)->hasRowSecurity; + /* Fall through to process lefthand args of SubLink */ } @@ -2040,7 +2058,7 @@ fireRIRonSubLink(Node *node, List *activeRIRs) * subselects of subselects for us. */ return expression_tree_walker(node, fireRIRonSubLink, - (void *) activeRIRs); + (void *) context); } @@ -2101,6 +2119,13 @@ fireRIRrules(Query *parsetree, List *activeRIRs) if (rte->rtekind == RTE_SUBQUERY || rte->rtekind == RTE_TABLEFUNCTION) { rte->subquery = fireRIRrules(rte->subquery, activeRIRs); + + /* + * While we are here, make sure the query is marked as having row + * security if any of its subqueries do. + */ + parsetree->hasRowSecurity |= rte->subquery->hasRowSecurity; + continue; } @@ -2218,6 +2243,12 @@ fireRIRrules(Query *parsetree, List *activeRIRs) cte->ctequery = (Node *) fireRIRrules((Query *) cte->ctequery, activeRIRs); + + /* + * While we are here, make sure the query is marked as having row + * security if any of its CTEs do. + */ + parsetree->hasRowSecurity |= ((Query *) cte->ctequery)->hasRowSecurity; } /* @@ -2225,9 +2256,22 @@ fireRIRrules(Query *parsetree, List *activeRIRs) * the rtable and cteList. */ if (parsetree->hasSubLinks) - query_tree_walker(parsetree, fireRIRonSubLink, (void *) activeRIRs, + { + fireRIRonSubLink_context context; + + context.activeRIRs = activeRIRs; + context.hasRowSecurity = false; + + query_tree_walker(parsetree, fireRIRonSubLink, (void *) &context, QTW_IGNORE_RC_SUBQUERIES); + /* + * Make sure the query is marked as having row security if any of its + * sublinks do. + */ + parsetree->hasRowSecurity |= context.hasRowSecurity; + } + /* * Apply any row-level security policies. We do this last because it * requires special recursion detection if the new quals have sublink @@ -2266,6 +2310,7 @@ fireRIRrules(Query *parsetree, List *activeRIRs) if (hasSubLinks) { acquireLocksOnSubLinks_context context; + fireRIRonSubLink_context fire_context; /* * Recursively process the new quals, checking for infinite @@ -2296,11 +2341,21 @@ fireRIRrules(Query *parsetree, List *activeRIRs) * Now that we have the locks on anything added by * get_row_security_policies, fire any RIR rules for them. */ + fire_context.activeRIRs = activeRIRs; + fire_context.hasRowSecurity = false; + expression_tree_walker((Node *) securityQuals, - fireRIRonSubLink, (void *) activeRIRs); + fireRIRonSubLink, (void *) &fire_context); expression_tree_walker((Node *) withCheckOptions, - fireRIRonSubLink, (void *) activeRIRs); + fireRIRonSubLink, (void *) &fire_context); + + /* + * We can ignore the value of fire_context.hasRowSecurity + * since we only reach this code in cases where hasRowSecurity + * is already true. + */ + Assert(hasRowSecurity); activeRIRs = list_delete_last(activeRIRs); } diff --git a/src/test/regress/expected/rowsecurity.out b/src/test/regress/expected/rowsecurity.out index 2fc722f5c41..e57da544f2b 100644 --- a/src/test/regress/expected/rowsecurity.out +++ b/src/test/regress/expected/rowsecurity.out @@ -4194,6 +4194,84 @@ execute q; --------------+--- (0 rows) +-- make sure RLS dependencies in CTEs are handled +reset role; +create or replace function rls_f() returns setof rls_t + stable language sql + as $$ with cte as (select * from rls_t) select * from cte $$; +prepare r as select current_user, * from rls_f(); +set role regress_rls_alice; +execute r; + current_user | c +-------------------+------------------ + regress_rls_alice | invisible to bob +(1 row) + +set role regress_rls_bob; +execute r; + current_user | c +--------------+--- +(0 rows) + +-- make sure RLS dependencies in subqueries are handled +reset role; +create or replace function rls_f() returns setof rls_t + stable language sql + as $$ select * from (select * from rls_t) _ $$; +prepare s as select current_user, * from rls_f(); +set role regress_rls_alice; +execute s; + current_user | c +-------------------+------------------ + regress_rls_alice | invisible to bob +(1 row) + +set role regress_rls_bob; +execute s; + current_user | c +--------------+--- +(0 rows) + +-- make sure RLS dependencies in sublinks are handled +reset role; +create or replace function rls_f() returns setof rls_t + stable language sql + as $$ select exists(select * from rls_t)::text $$; +prepare t as select current_user, * from rls_f(); +set role regress_rls_alice; +execute t; + current_user | c +-------------------+------ + regress_rls_alice | true +(1 row) + +set role regress_rls_bob; +execute t; + current_user | c +-----------------+------- + regress_rls_bob | false +(1 row) + +-- make sure RLS dependencies are handled when coercion projections are inserted +reset role; +create or replace function rls_f() returns setof rls_t + stable language sql + as $$ select * from (select array_agg(c) as cs from rls_t) _ group by cs $$; +prepare u as select current_user, * from rls_f(); +set role regress_rls_alice; +execute u; + current_user | c +-------------------+---------------------- + regress_rls_alice | {"invisible to bob"} +(1 row) + +set role regress_rls_bob; +execute u; + current_user | c +-----------------+--- + regress_rls_bob | +(1 row) + RESET ROLE; DROP FUNCTION rls_f(); DROP TABLE rls_t; diff --git a/src/test/regress/sql/rowsecurity.sql b/src/test/regress/sql/rowsecurity.sql index e828368a4e5..225086be62e 100644 --- a/src/test/regress/sql/rowsecurity.sql +++ b/src/test/regress/sql/rowsecurity.sql @@ -1884,6 +1884,50 @@ execute q; set role regress_rls_bob; execute q; +-- make sure RLS dependencies in CTEs are handled +reset role; +create or replace function rls_f() returns setof rls_t + stable language sql + as $$ with cte as (select * from rls_t) select * from cte $$; +prepare r as select current_user, * from rls_f(); +set role regress_rls_alice; +execute r; +set role regress_rls_bob; +execute r; + +-- make sure RLS dependencies in subqueries are handled +reset role; +create or replace function rls_f() returns setof rls_t + stable language sql + as $$ select * from (select * from rls_t) _ $$; +prepare s as select current_user, * from rls_f(); +set role regress_rls_alice; +execute s; +set role regress_rls_bob; +execute s; + +-- make sure RLS dependencies in sublinks are handled +reset role; +create or replace function rls_f() returns setof rls_t + stable language sql + as $$ select exists(select * from rls_t)::text $$; +prepare t as select current_user, * from rls_f(); +set role regress_rls_alice; +execute t; +set role regress_rls_bob; +execute t; + +-- make sure RLS dependencies are handled when coercion projections are inserted +reset role; +create or replace function rls_f() returns setof rls_t + stable language sql + as $$ select * from (select array_agg(c) as cs from rls_t) _ group by cs $$; +prepare u as select current_user, * from rls_f(); +set role regress_rls_alice; +execute u; +set role regress_rls_bob; +execute u; + RESET ROLE; DROP FUNCTION rls_f(); DROP TABLE rls_t; diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list index 2eb501b6e74..fe36db36936 100644 --- a/src/tools/pgindent/typedefs.list +++ b/src/tools/pgindent/typedefs.list @@ -3133,6 +3133,7 @@ fill_string_relopt finalize_primnode_context find_dependent_phvs_context find_expr_references_context +fireRIRonSubLink_context fix_join_expr_context fix_scan_expr_context fix_upper_expr_context --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
