Fix ExecCheckPermissions call in RI_Initial_Check

RI_Initial_Check was setting up a list of RTEPermissionInfo for
ExecCheckPermissions() wrong, and the problem is subtle enough that it
doesn't have any immediate effect in core code.  However, if an
extension is using the ExecutorCheckPerms_hook, then it would get the
wrong parameters and perhaps arrive at a wrong conclusion, or outright
malfunction.  Fix by constructing that list and the RTE list more
honestly.

We also add an assertion check to verify that these lists match.  This
new assertion would have caught this bug.

Co-authored-by: Олег Целебровский (Oleg Tselebrovskii) <[email protected]>
Co-authored-by: Álvaro Herrera <[email protected]>
Reviewed-by: Amit Langote <[email protected]>
Discussion: https://postgr.es/m/3722b7a2cbe27a1796ee40824bd86dd1@postgrespro.ru

diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c
index 00a1f158d8ef12f6a5ca037f278fe2a62228dfbf..0186be452cb2c6933b6c9e732380bef4e0524e1e 100644 (file)
--- a/src/backend/executor/execMain.c
+++ b/src/backend/executor/execMain.c
@@ -584,6 +584,28 @@ ExecCheckPermissions(List *rangeTable, List *rteperminfos,
        ListCell   *l;
        bool            result = true;
 
+#ifdef USE_ASSERT_CHECKING
+       Bitmapset  *indexset = NULL;
+
+       /* Check that rteperminfos is consistent with rangeTable */
+       foreach(l, rangeTable)
+       {
+               RangeTblEntry *rte = lfirst_node(RangeTblEntry, l);
+
+               if (rte->perminfoindex != 0)
+               {
+                       /* Sanity checks */
+                       (void) getRTEPermissionInfo(rteperminfos, rte);
+                       /* Many-to-one mapping not allowed */
+                       Assert(!bms_is_member(rte->perminfoindex, indexset));
+                       indexset = bms_add_member(indexset, rte->perminfoindex);
+               }
+       }
+
+       /* All rteperminfos are referenced */
+       Assert(bms_num_members(indexset) == list_length(rteperminfos));
+#endif
+
        foreach(l, rteperminfos)
        {
                RTEPermissionInfo *perminfo = lfirst_node(RTEPermissionInfo, l);

diff --git a/src/backend/utils/adt/ri_triggers.c b/src/backend/utils/adt/ri_triggers.c
index 375b17b9f3bb17c6584e69f58457e5079877de77..6945d99b3d5f2ca7dc4f2e1d6daf745d2585656b 100644 (file)
--- a/src/backend/utils/adt/ri_triggers.c
+++ b/src/backend/utils/adt/ri_triggers.c
@@ -1373,10 +1373,11 @@ RI_Initial_Check(Trigger *trigger, Relation fk_rel, Relation pk_rel)
        char            fkrelname[MAX_QUOTED_REL_NAME_LEN];
        char            pkattname[MAX_QUOTED_NAME_LEN + 3];
        char            fkattname[MAX_QUOTED_NAME_LEN + 3];
-       RangeTblEntry *pkrte;
-       RangeTblEntry *fkrte;
+       RangeTblEntry *rte;
        RTEPermissionInfo *pk_perminfo;
        RTEPermissionInfo *fk_perminfo;
+       List       *rtes = NIL;
+       List       *perminfos = NIL;
        const char *sep;
        const char *fk_only;
        const char *pk_only;
@@ -1394,25 +1395,29 @@ RI_Initial_Check(Trigger *trigger, Relation fk_rel, Relation pk_rel)
         *
         * XXX are there any other show-stopper conditions to check?
         */
-       pkrte = makeNode(RangeTblEntry);
-       pkrte->rtekind = RTE_RELATION;
-       pkrte->relid = RelationGetRelid(pk_rel);
-       pkrte->relkind = pk_rel->rd_rel->relkind;
-       pkrte->rellockmode = AccessShareLock;
-
        pk_perminfo = makeNode(RTEPermissionInfo);
        pk_perminfo->relid = RelationGetRelid(pk_rel);
        pk_perminfo->requiredPerms = ACL_SELECT;
-
-       fkrte = makeNode(RangeTblEntry);
-       fkrte->rtekind = RTE_RELATION;
-       fkrte->relid = RelationGetRelid(fk_rel);
-       fkrte->relkind = fk_rel->rd_rel->relkind;
-       fkrte->rellockmode = AccessShareLock;
+       perminfos = lappend(perminfos, pk_perminfo);
+       rte = makeNode(RangeTblEntry);
+       rte->rtekind = RTE_RELATION;
+       rte->relid = RelationGetRelid(pk_rel);
+       rte->relkind = pk_rel->rd_rel->relkind;
+       rte->rellockmode = AccessShareLock;
+       rte->perminfoindex = list_length(perminfos);
+       rtes = lappend(rtes, rte);
 
        fk_perminfo = makeNode(RTEPermissionInfo);
        fk_perminfo->relid = RelationGetRelid(fk_rel);
        fk_perminfo->requiredPerms = ACL_SELECT;
+       perminfos = lappend(perminfos, fk_perminfo);
+       rte = makeNode(RangeTblEntry);
+       rte->rtekind = RTE_RELATION;
+       rte->relid = RelationGetRelid(fk_rel);
+       rte->relkind = fk_rel->rd_rel->relkind;
+       rte->rellockmode = AccessShareLock;
+       rte->perminfoindex = list_length(perminfos);
+       rtes = lappend(rtes, rte);
 
        for (int i = 0; i < riinfo->nkeys; i++)
        {
@@ -1425,8 +1430,7 @@ RI_Initial_Check(Trigger *trigger, Relation fk_rel, Relation pk_rel)
                fk_perminfo->selectedCols = bms_add_member(fk_perminfo->selectedCols, attno);
        }
 
-       if (!ExecCheckPermissions(list_make2(fkrte, pkrte),
-                                                         list_make2(fk_perminfo, pk_perminfo), false))
+       if (!ExecCheckPermissions(rtes, perminfos, false))
                return false;
 
        /*
@@ -1436,9 +1440,11 @@ RI_Initial_Check(Trigger *trigger, Relation fk_rel, Relation pk_rel)
         */
        if (!has_bypassrls_privilege(GetUserId()) &&
                ((pk_rel->rd_rel->relrowsecurity &&
-                 !object_ownercheck(RelationRelationId, pkrte->relid, GetUserId())) ||
+                 !object_ownercheck(RelationRelationId, RelationGetRelid(pk_rel),
+                                                        GetUserId())) ||
                 (fk_rel->rd_rel->relrowsecurity &&
-                 !object_ownercheck(RelationRelationId, fkrte->relid, GetUserId()))))
+                 !object_ownercheck(RelationRelationId, RelationGetRelid(fk_rel),
+                                                        GetUserId()))))
                return false;
 
        /*----------