From 6d429165eda81f1c8e1dc1781e5b571788293dcc Mon Sep 17 00:00:00 2001 From: amitlan Date: Wed, 30 Jun 2021 20:08:25 +0900 Subject: [PATCH] Explicitly track RT indexes of relations to check permissions of --- src/backend/commands/copy.c | 2 +- src/backend/executor/execMain.c | 14 ++++++++------ src/backend/executor/execParallel.c | 1 + src/backend/nodes/copyfuncs.c | 2 ++ src/backend/nodes/equalfuncs.c | 1 + src/backend/nodes/outfuncs.c | 2 ++ src/backend/nodes/readfuncs.c | 2 ++ src/backend/optimizer/plan/planner.c | 1 + src/backend/optimizer/plan/setrefs.c | 12 ++++++++++++ src/backend/optimizer/prep/prepjointree.c | 2 ++ src/backend/parser/analyze.c | 8 ++++++++ src/backend/parser/parse_relation.c | 11 +++++++++++ src/backend/parser/parse_utilcmd.c | 1 + src/backend/rewrite/rewriteHandler.c | 12 ++++++++++++ src/backend/rewrite/rewriteManip.c | 2 ++ src/backend/utils/adt/ri_triggers.c | 4 +++- src/include/executor/executor.h | 3 ++- src/include/nodes/parsenodes.h | 2 ++ src/include/nodes/plannodes.h | 3 +++ src/include/parser/parse_node.h | 2 ++ 20 files changed, 78 insertions(+), 9 deletions(-) diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c index 8265b981eb..500dc51b15 100644 --- a/src/backend/commands/copy.c +++ b/src/backend/commands/copy.c @@ -158,7 +158,7 @@ DoCopy(ParseState *pstate, const CopyStmt *stmt, else rte->selectedCols = bms_add_member(rte->selectedCols, attno); } - ExecCheckRTPerms(pstate->p_rtable, true); + ExecCheckRTPerms(pstate->p_rtable, bms_make_singleton(1), true); /* * Permission check for row security policies. diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c index b3ce4bae53..8ed6dd6235 100644 --- a/src/backend/executor/execMain.c +++ b/src/backend/executor/execMain.c @@ -553,7 +553,7 @@ ExecutorRewind(QueryDesc *queryDesc) /* * ExecCheckRTPerms - * Check access permissions for all relations listed in a range table. + * Check access permissions for the specified relations in a range table. * * Returns true if permissions are adequate. Otherwise, throws an appropriate * error if ereport_on_violation is true, or simply returns false otherwise. @@ -565,14 +565,16 @@ ExecutorRewind(QueryDesc *queryDesc) * See rewrite/rowsecurity.c. */ bool -ExecCheckRTPerms(List *rangeTable, bool ereport_on_violation) +ExecCheckRTPerms(List *rangeTable, Bitmapset *checkPermRels, + bool ereport_on_violation) { - ListCell *l; + int rti; bool result = true; - foreach(l, rangeTable) + rti = -1; + while ((rti = bms_next_member(checkPermRels, rti)) > 0) { - RangeTblEntry *rte = (RangeTblEntry *) lfirst(l); + RangeTblEntry *rte = (RangeTblEntry *) list_nth(rangeTable, rti - 1); result = ExecCheckRTEPerms(rte); if (!result) @@ -815,7 +817,7 @@ InitPlan(QueryDesc *queryDesc, int eflags) /* * Do permissions checks */ - ExecCheckRTPerms(rangeTable, true); + ExecCheckRTPerms(rangeTable, plannedstmt->checkPermRels, true); /* * initialize the node's execution state diff --git a/src/backend/executor/execParallel.c b/src/backend/executor/execParallel.c index 12c41d746b..21971f1e10 100644 --- a/src/backend/executor/execParallel.c +++ b/src/backend/executor/execParallel.c @@ -184,6 +184,7 @@ ExecSerializePlan(Plan *plan, EState *estate) pstmt->parallelModeNeeded = false; pstmt->planTree = plan; pstmt->rtable = estate->es_range_table; + pstmt->checkPermRels = NULL; pstmt->resultRelations = NIL; pstmt->appendRelations = NIL; diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c index bd87f23784..f9241f56ca 100644 --- a/src/backend/nodes/copyfuncs.c +++ b/src/backend/nodes/copyfuncs.c @@ -90,6 +90,7 @@ _copyPlannedStmt(const PlannedStmt *from) COPY_SCALAR_FIELD(jitFlags); COPY_NODE_FIELD(planTree); COPY_NODE_FIELD(rtable); + COPY_BITMAPSET_FIELD(checkPermRels); COPY_NODE_FIELD(resultRelations); COPY_NODE_FIELD(appendRelations); COPY_NODE_FIELD(subplans); @@ -3176,6 +3177,7 @@ _copyQuery(const Query *from) COPY_SCALAR_FIELD(isReturn); COPY_NODE_FIELD(cteList); COPY_NODE_FIELD(rtable); + COPY_BITMAPSET_FIELD(checkPermRels); COPY_NODE_FIELD(jointree); COPY_NODE_FIELD(targetList); COPY_SCALAR_FIELD(override); diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c index dba3e6b31e..0313b2a469 100644 --- a/src/backend/nodes/equalfuncs.c +++ b/src/backend/nodes/equalfuncs.c @@ -979,6 +979,7 @@ _equalQuery(const Query *a, const Query *b) COMPARE_SCALAR_FIELD(isReturn); COMPARE_NODE_FIELD(cteList); COMPARE_NODE_FIELD(rtable); + COMPARE_BITMAPSET_FIELD(checkPermRels); COMPARE_NODE_FIELD(jointree); COMPARE_NODE_FIELD(targetList); COMPARE_SCALAR_FIELD(override); diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c index e32b92e299..7bee62dc75 100644 --- a/src/backend/nodes/outfuncs.c +++ b/src/backend/nodes/outfuncs.c @@ -308,6 +308,7 @@ _outPlannedStmt(StringInfo str, const PlannedStmt *node) WRITE_INT_FIELD(jitFlags); WRITE_NODE_FIELD(planTree); WRITE_NODE_FIELD(rtable); + WRITE_BITMAPSET_FIELD(checkPermRels); WRITE_NODE_FIELD(resultRelations); WRITE_NODE_FIELD(appendRelations); WRITE_NODE_FIELD(subplans); @@ -3071,6 +3072,7 @@ _outQuery(StringInfo str, const Query *node) WRITE_BOOL_FIELD(isReturn); WRITE_NODE_FIELD(cteList); WRITE_NODE_FIELD(rtable); + WRITE_BITMAPSET_FIELD(checkPermRels); WRITE_NODE_FIELD(jointree); WRITE_NODE_FIELD(targetList); WRITE_ENUM_FIELD(override, OverridingKind); diff --git a/src/backend/nodes/readfuncs.c b/src/backend/nodes/readfuncs.c index f0b34ecfac..f0f80b5ac4 100644 --- a/src/backend/nodes/readfuncs.c +++ b/src/backend/nodes/readfuncs.c @@ -266,6 +266,7 @@ _readQuery(void) READ_BOOL_FIELD(isReturn); READ_NODE_FIELD(cteList); READ_NODE_FIELD(rtable); + READ_BITMAPSET_FIELD(checkPermRels); READ_NODE_FIELD(jointree); READ_NODE_FIELD(targetList); READ_ENUM_FIELD(override, OverridingKind); @@ -1589,6 +1590,7 @@ _readPlannedStmt(void) READ_INT_FIELD(jitFlags); READ_NODE_FIELD(planTree); READ_NODE_FIELD(rtable); + READ_BITMAPSET_FIELD(checkPermRels); READ_NODE_FIELD(resultRelations); READ_NODE_FIELD(appendRelations); READ_NODE_FIELD(subplans); diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c index 1868c4eff4..ce8484fe21 100644 --- a/src/backend/optimizer/plan/planner.c +++ b/src/backend/optimizer/plan/planner.c @@ -513,6 +513,7 @@ standard_planner(Query *parse, const char *query_string, int cursorOptions, result->parallelModeNeeded = glob->parallelModeNeeded; result->planTree = top_plan; result->rtable = glob->finalrtable; + result->checkPermRels = parse->checkPermRels; result->resultRelations = glob->resultRelations; result->appendRelations = glob->appendRelations; result->subplans = glob->subplans; diff --git a/src/backend/optimizer/plan/setrefs.c b/src/backend/optimizer/plan/setrefs.c index 61ccfd300b..592de92aba 100644 --- a/src/backend/optimizer/plan/setrefs.c +++ b/src/backend/optimizer/plan/setrefs.c @@ -360,6 +360,8 @@ add_rtes_to_flat_rtable(PlannerInfo *root, bool recursing) if (rel != NULL) { + int rtoff = list_length(glob->finalrtable); + Assert(rel->relid == rti); /* sanity check on array */ /* @@ -387,6 +389,16 @@ add_rtes_to_flat_rtable(PlannerInfo *root, bool recursing) IS_DUMMY_REL(fetch_upper_rel(rel->subroot, UPPERREL_FINAL, NULL))) add_rtes_to_flat_rtable(rel->subroot, true); + + /* + * Pull up the checkPermRels set of subqueries that themselves + * were not. + */ + Assert(rte->subquery != NULL); + root->parse->checkPermRels = + bms_union(root->parse->checkPermRels, + offset_relid_set(rte->subquery->checkPermRels, + rtoff)); } } rti++; diff --git a/src/backend/optimizer/prep/prepjointree.c b/src/backend/optimizer/prep/prepjointree.c index 62a1668796..ec057e1335 100644 --- a/src/backend/optimizer/prep/prepjointree.c +++ b/src/backend/optimizer/prep/prepjointree.c @@ -1132,6 +1132,8 @@ pull_up_simple_subquery(PlannerInfo *root, Node *jtnode, RangeTblEntry *rte, */ parse->rtable = list_concat(parse->rtable, subquery->rtable); + parse->checkPermRels = bms_union(parse->checkPermRels, subquery->checkPermRels); + /* * Pull up any FOR UPDATE/SHARE markers, too. (OffsetVarNodes already * adjusted the marker rtindexes, so just concat the lists.) diff --git a/src/backend/parser/analyze.c b/src/backend/parser/analyze.c index 438b077004..d698191b0c 100644 --- a/src/backend/parser/analyze.c +++ b/src/backend/parser/analyze.c @@ -475,6 +475,7 @@ transformDeleteStmt(ParseState *pstate, DeleteStmt *stmt) /* done building the range table and jointree */ qry->rtable = pstate->p_rtable; + qry->checkPermRels= pstate->p_check_perm_rels; qry->jointree = makeFromExpr(pstate->p_joinlist, qual); qry->hasSubLinks = pstate->p_hasSubLinks; @@ -895,6 +896,7 @@ transformInsertStmt(ParseState *pstate, InsertStmt *stmt) /* done building the range table and jointree */ qry->rtable = pstate->p_rtable; + qry->checkPermRels= pstate->p_check_perm_rels; qry->jointree = makeFromExpr(pstate->p_joinlist, NULL); qry->hasTargetSRFs = pstate->p_hasTargetSRFs; @@ -1348,6 +1350,7 @@ transformSelectStmt(ParseState *pstate, SelectStmt *stmt) resolveTargetListUnknowns(pstate, qry->targetList); qry->rtable = pstate->p_rtable; + qry->checkPermRels= pstate->p_check_perm_rels; qry->jointree = makeFromExpr(pstate->p_joinlist, qual); qry->hasSubLinks = pstate->p_hasSubLinks; @@ -1582,6 +1585,7 @@ transformValuesClause(ParseState *pstate, SelectStmt *stmt) linitial(stmt->lockingClause))->strength)))); qry->rtable = pstate->p_rtable; + qry->checkPermRels= pstate->p_check_perm_rels; qry->jointree = makeFromExpr(pstate->p_joinlist, NULL); qry->hasSubLinks = pstate->p_hasSubLinks; @@ -1828,6 +1832,7 @@ transformSetOperationStmt(ParseState *pstate, SelectStmt *stmt) qry->limitOption = stmt->limitOption; qry->rtable = pstate->p_rtable; + qry->checkPermRels= pstate->p_check_perm_rels; qry->jointree = makeFromExpr(pstate->p_joinlist, NULL); qry->hasSubLinks = pstate->p_hasSubLinks; @@ -2286,6 +2291,7 @@ transformReturnStmt(ParseState *pstate, ReturnStmt *stmt) if (pstate->p_resolve_unknowns) resolveTargetListUnknowns(pstate, qry->targetList); qry->rtable = pstate->p_rtable; + qry->checkPermRels= pstate->p_check_perm_rels; qry->jointree = makeFromExpr(pstate->p_joinlist, NULL); qry->hasSubLinks = pstate->p_hasSubLinks; qry->hasWindowFuncs = pstate->p_hasWindowFuncs; @@ -2352,6 +2358,7 @@ transformUpdateStmt(ParseState *pstate, UpdateStmt *stmt) qry->targetList = transformUpdateTargetList(pstate, stmt->targetList); qry->rtable = pstate->p_rtable; + qry->checkPermRels= pstate->p_check_perm_rels; qry->jointree = makeFromExpr(pstate->p_joinlist, qual); qry->hasTargetSRFs = pstate->p_hasTargetSRFs; @@ -2711,6 +2718,7 @@ transformPLAssignStmt(ParseState *pstate, PLAssignStmt *stmt) &qry->targetList); qry->rtable = pstate->p_rtable; + qry->checkPermRels= pstate->p_check_perm_rels; qry->jointree = makeFromExpr(pstate->p_joinlist, qual); qry->hasSubLinks = pstate->p_hasSubLinks; diff --git a/src/backend/parser/parse_relation.c b/src/backend/parser/parse_relation.c index 7465919044..7479e2c5a2 100644 --- a/src/backend/parser/parse_relation.c +++ b/src/backend/parser/parse_relation.c @@ -1464,6 +1464,8 @@ addRangeTableEntry(ParseState *pstate, * appropriate. */ pstate->p_rtable = lappend(pstate->p_rtable, rte); + pstate->p_check_perm_rels = bms_add_member(pstate->p_check_perm_rels, + list_length(pstate->p_rtable)); /* * Build a ParseNamespaceItem, but don't add it to the pstate's namespace @@ -1552,6 +1554,8 @@ addRangeTableEntryForRelation(ParseState *pstate, * appropriate. */ pstate->p_rtable = lappend(pstate->p_rtable, rte); + pstate->p_check_perm_rels = bms_add_member(pstate->p_check_perm_rels, + list_length(pstate->p_rtable)); /* * Build a ParseNamespaceItem, but don't add it to the pstate's namespace @@ -1649,6 +1653,7 @@ addRangeTableEntryForSubquery(ParseState *pstate, * appropriate. */ pstate->p_rtable = lappend(pstate->p_rtable, rte); + /* No need to add this RTE's index to pstate->p_check_perm_rels. */ /* * Build a ParseNamespaceItem, but don't add it to the pstate's namespace @@ -1955,6 +1960,7 @@ addRangeTableEntryForFunction(ParseState *pstate, * appropriate. */ pstate->p_rtable = lappend(pstate->p_rtable, rte); + /* No need to add this RTE's index to pstate->p_check_perm_rels. */ /* * Build a ParseNamespaceItem, but don't add it to the pstate's namespace @@ -2026,6 +2032,7 @@ addRangeTableEntryForTableFunc(ParseState *pstate, * appropriate. */ pstate->p_rtable = lappend(pstate->p_rtable, rte); + /* No need to add this RTE's index to pstate->p_check_perm_rels. */ /* * Build a ParseNamespaceItem, but don't add it to the pstate's namespace @@ -2113,6 +2120,7 @@ addRangeTableEntryForValues(ParseState *pstate, * appropriate. */ pstate->p_rtable = lappend(pstate->p_rtable, rte); + /* No need to add this RTE's index to pstate->p_check_perm_rels. */ /* * Build a ParseNamespaceItem, but don't add it to the pstate's namespace @@ -2204,6 +2212,7 @@ addRangeTableEntryForJoin(ParseState *pstate, * appropriate. */ pstate->p_rtable = lappend(pstate->p_rtable, rte); + /* No need to add this RTE's index to pstate->p_check_perm_rels. */ /* * Build a ParseNamespaceItem, but don't add it to the pstate's namespace @@ -2354,6 +2363,7 @@ addRangeTableEntryForCTE(ParseState *pstate, * appropriate. */ pstate->p_rtable = lappend(pstate->p_rtable, rte); + /* No need to add this RTE's index to pstate->p_check_perm_rels. */ /* * Build a ParseNamespaceItem, but don't add it to the pstate's namespace @@ -2477,6 +2487,7 @@ addRangeTableEntryForENR(ParseState *pstate, * appropriate. */ pstate->p_rtable = lappend(pstate->p_rtable, rte); + /* No need to add this RTE's index to pstate->p_check_perm_rels. */ /* * Build a ParseNamespaceItem, but don't add it to the pstate's namespace diff --git a/src/backend/parser/parse_utilcmd.c b/src/backend/parser/parse_utilcmd.c index 81d3e7990c..65385d310a 100644 --- a/src/backend/parser/parse_utilcmd.c +++ b/src/backend/parser/parse_utilcmd.c @@ -3069,6 +3069,7 @@ transformRuleStmt(RuleStmt *stmt, const char *queryString, nothing_qry->commandType = CMD_NOTHING; nothing_qry->rtable = pstate->p_rtable; + nothing_qry->checkPermRels = pstate->p_check_perm_rels; nothing_qry->jointree = makeFromExpr(NIL, NULL); /* no join wanted */ *actions = list_make1(nothing_qry); diff --git a/src/backend/rewrite/rewriteHandler.c b/src/backend/rewrite/rewriteHandler.c index 88a9e95e33..1b393d5710 100644 --- a/src/backend/rewrite/rewriteHandler.c +++ b/src/backend/rewrite/rewriteHandler.c @@ -420,6 +420,8 @@ rewriteRuleAction(Query *parsetree, */ sub_action->rtable = list_concat(copyObject(parsetree->rtable), sub_action->rtable); + sub_action->checkPermRels = bms_union(parsetree->checkPermRels, + sub_action->checkPermRels); /* * There could have been some SubLinks in parsetree's rtable, in which @@ -1720,6 +1722,9 @@ ApplyRetrieveRule(Query *parsetree, rte = rt_fetch(rt_index, parsetree->rtable); newrte = copyObject(rte); parsetree->rtable = lappend(parsetree->rtable, newrte); + parsetree->checkPermRels = + bms_add_member(parsetree->checkPermRels, + list_length(parsetree->rtable)); parsetree->resultRelation = list_length(parsetree->rtable); /* @@ -1842,6 +1847,11 @@ ApplyRetrieveRule(Query *parsetree, rte->updatedCols = NULL; rte->extraUpdatedCols = NULL; + /* Update checkPermRels set in the respective Query nodes. */ + parsetree->checkPermRels = bms_del_member(parsetree->checkPermRels, rt_index); + rule_action->checkPermRels = bms_add_member(rule_action->checkPermRels, + PRS2_OLD_VARNO); + return parsetree; } @@ -3195,6 +3205,8 @@ rewriteTargetView(Query *parsetree, Relation view) parsetree->rtable = lappend(parsetree->rtable, new_rte); new_rt_index = list_length(parsetree->rtable); + parsetree->checkPermRels = bms_add_member(parsetree->checkPermRels, + new_rt_index); /* * INSERTs never inherit. For UPDATE/DELETE, we use the view query's diff --git a/src/backend/rewrite/rewriteManip.c b/src/backend/rewrite/rewriteManip.c index d4e0b8b4de..0a0f5f2d5c 100644 --- a/src/backend/rewrite/rewriteManip.c +++ b/src/backend/rewrite/rewriteManip.c @@ -461,6 +461,8 @@ OffsetVarNodes(Node *node, int offset, int sublevels_up) rc->rti += offset; } + + qry->checkPermRels = offset_relid_set(qry->checkPermRels, offset); } query_tree_walker(qry, OffsetVarNodes_walker, (void *) &context, 0); diff --git a/src/backend/utils/adt/ri_triggers.c b/src/backend/utils/adt/ri_triggers.c index 96269fc2ad..83536afa23 100644 --- a/src/backend/utils/adt/ri_triggers.c +++ b/src/backend/utils/adt/ri_triggers.c @@ -1352,7 +1352,9 @@ RI_Initial_Check(Trigger *trigger, Relation fk_rel, Relation pk_rel) fkrte->selectedCols = bms_add_member(fkrte->selectedCols, attno); } - if (!ExecCheckRTPerms(list_make2(fkrte, pkrte), false)) + if (!ExecCheckRTPerms(list_make2(fkrte, pkrte), + bms_add_member(bms_make_singleton(1), 2), + false)) return false; /* diff --git a/src/include/executor/executor.h b/src/include/executor/executor.h index 3dc03c913e..0e679b66b5 100644 --- a/src/include/executor/executor.h +++ b/src/include/executor/executor.h @@ -196,7 +196,8 @@ extern void standard_ExecutorFinish(QueryDesc *queryDesc); extern void ExecutorEnd(QueryDesc *queryDesc); extern void standard_ExecutorEnd(QueryDesc *queryDesc); extern void ExecutorRewind(QueryDesc *queryDesc); -extern bool ExecCheckRTPerms(List *rangeTable, bool ereport_on_violation); +extern bool ExecCheckRTPerms(List *rangeTable, Bitmapset *checkPermRels, + bool ereport_on_violation); extern void CheckValidResultRel(ResultRelInfo *resultRelInfo, CmdType operation); extern void InitResultRelInfo(ResultRelInfo *resultRelInfo, Relation resultRelationDesc, diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h index def9651b34..a27fd30093 100644 --- a/src/include/nodes/parsenodes.h +++ b/src/include/nodes/parsenodes.h @@ -145,6 +145,8 @@ typedef struct Query List *cteList; /* WITH list (of CommonTableExpr's) */ List *rtable; /* list of range table entries */ + Bitmapset *checkPermRels; /* range table indexes of relations to check + * the permissions of */ FromExpr *jointree; /* table join tree (FROM and WHERE clauses) */ List *targetList; /* target list (of TargetEntry) */ diff --git a/src/include/nodes/plannodes.h b/src/include/nodes/plannodes.h index aaa3b65d04..1b90c38034 100644 --- a/src/include/nodes/plannodes.h +++ b/src/include/nodes/plannodes.h @@ -65,6 +65,9 @@ typedef struct PlannedStmt List *rtable; /* list of RangeTblEntry nodes */ + Bitmapset *checkPermRels; /* range table indexes of relations to check + * the permissions of */ + /* rtable indexes of target relations for INSERT/UPDATE/DELETE */ List *resultRelations; /* integer list of RT indexes, or NIL */ diff --git a/src/include/parser/parse_node.h b/src/include/parser/parse_node.h index 1500de2dd0..6b93988eb9 100644 --- a/src/include/parser/parse_node.h +++ b/src/include/parser/parse_node.h @@ -180,6 +180,8 @@ struct ParseState ParseState *parentParseState; /* stack link */ const char *p_sourcetext; /* source text, or NULL if not available */ List *p_rtable; /* range table so far */ + Bitmapset *p_check_perm_rels; /* range table indexes of relations to + * check the permissions of */ List *p_joinexprs; /* JoinExprs for RTE_JOIN p_rtable entries */ List *p_joinlist; /* join items so far (will become FromExpr * node's fromlist) */ -- 2.24.1