Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit f75cec4

Browse files
alvherreОлег Целебровский (Oleg Tselebrovskii)
and
Олег Целебровский (Oleg Tselebrovskii)
committed
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) <o.tselebrovskiy@postgrespro.ru> Co-authored-by: Álvaro Herrera <alvherre@alvh.no-ip.org> Reviewed-by: Amit Langote <amitlangote09@gmail.com> Discussion: https://postgr.es/m/3722b7a2cbe27a1796ee40824bd86dd1@postgrespro.ru
1 parent 4c40995 commit f75cec4

File tree

2 files changed

+46
-18
lines changed

2 files changed

+46
-18
lines changed

src/backend/executor/execMain.c

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -584,6 +584,28 @@ ExecCheckPermissions(List *rangeTable, List *rteperminfos,
584584
ListCell *l;
585585
bool result = true;
586586

587+
#ifdef USE_ASSERT_CHECKING
588+
Bitmapset *indexset = NULL;
589+
590+
/* Check that rteperminfos is consistent with rangeTable */
591+
foreach(l, rangeTable)
592+
{
593+
RangeTblEntry *rte = lfirst_node(RangeTblEntry, l);
594+
595+
if (rte->perminfoindex != 0)
596+
{
597+
/* Sanity checks */
598+
(void) getRTEPermissionInfo(rteperminfos, rte);
599+
/* Many-to-one mapping not allowed */
600+
Assert(!bms_is_member(rte->perminfoindex, indexset));
601+
indexset = bms_add_member(indexset, rte->perminfoindex);
602+
}
603+
}
604+
605+
/* All rteperminfos are referenced */
606+
Assert(bms_num_members(indexset) == list_length(rteperminfos));
607+
#endif
608+
587609
foreach(l, rteperminfos)
588610
{
589611
RTEPermissionInfo *perminfo = lfirst_node(RTEPermissionInfo, l);

src/backend/utils/adt/ri_triggers.c

Lines changed: 24 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1373,10 +1373,11 @@ RI_Initial_Check(Trigger *trigger, Relation fk_rel, Relation pk_rel)
13731373
char fkrelname[MAX_QUOTED_REL_NAME_LEN];
13741374
char pkattname[MAX_QUOTED_NAME_LEN + 3];
13751375
char fkattname[MAX_QUOTED_NAME_LEN + 3];
1376-
RangeTblEntry *pkrte;
1377-
RangeTblEntry *fkrte;
1376+
RangeTblEntry *rte;
13781377
RTEPermissionInfo *pk_perminfo;
13791378
RTEPermissionInfo *fk_perminfo;
1379+
List *rtes = NIL;
1380+
List *perminfos = NIL;
13801381
const char *sep;
13811382
const char *fk_only;
13821383
const char *pk_only;
@@ -1394,25 +1395,29 @@ RI_Initial_Check(Trigger *trigger, Relation fk_rel, Relation pk_rel)
13941395
*
13951396
* XXX are there any other show-stopper conditions to check?
13961397
*/
1397-
pkrte = makeNode(RangeTblEntry);
1398-
pkrte->rtekind = RTE_RELATION;
1399-
pkrte->relid = RelationGetRelid(pk_rel);
1400-
pkrte->relkind = pk_rel->rd_rel->relkind;
1401-
pkrte->rellockmode = AccessShareLock;
1402-
14031398
pk_perminfo = makeNode(RTEPermissionInfo);
14041399
pk_perminfo->relid = RelationGetRelid(pk_rel);
14051400
pk_perminfo->requiredPerms = ACL_SELECT;
1406-
1407-
fkrte = makeNode(RangeTblEntry);
1408-
fkrte->rtekind = RTE_RELATION;
1409-
fkrte->relid = RelationGetRelid(fk_rel);
1410-
fkrte->relkind = fk_rel->rd_rel->relkind;
1411-
fkrte->rellockmode = AccessShareLock;
1401+
perminfos = lappend(perminfos, pk_perminfo);
1402+
rte = makeNode(RangeTblEntry);
1403+
rte->rtekind = RTE_RELATION;
1404+
rte->relid = RelationGetRelid(pk_rel);
1405+
rte->relkind = pk_rel->rd_rel->relkind;
1406+
rte->rellockmode = AccessShareLock;
1407+
rte->perminfoindex = list_length(perminfos);
1408+
rtes = lappend(rtes, rte);
14121409

14131410
fk_perminfo = makeNode(RTEPermissionInfo);
14141411
fk_perminfo->relid = RelationGetRelid(fk_rel);
14151412
fk_perminfo->requiredPerms = ACL_SELECT;
1413+
perminfos = lappend(perminfos, fk_perminfo);
1414+
rte = makeNode(RangeTblEntry);
1415+
rte->rtekind = RTE_RELATION;
1416+
rte->relid = RelationGetRelid(fk_rel);
1417+
rte->relkind = fk_rel->rd_rel->relkind;
1418+
rte->rellockmode = AccessShareLock;
1419+
rte->perminfoindex = list_length(perminfos);
1420+
rtes = lappend(rtes, rte);
14161421

14171422
for (int i = 0; i < riinfo->nkeys; i++)
14181423
{
@@ -1425,8 +1430,7 @@ RI_Initial_Check(Trigger *trigger, Relation fk_rel, Relation pk_rel)
14251430
fk_perminfo->selectedCols = bms_add_member(fk_perminfo->selectedCols, attno);
14261431
}
14271432

1428-
if (!ExecCheckPermissions(list_make2(fkrte, pkrte),
1429-
list_make2(fk_perminfo, pk_perminfo), false))
1433+
if (!ExecCheckPermissions(rtes, perminfos, false))
14301434
return false;
14311435

14321436
/*
@@ -1436,9 +1440,11 @@ RI_Initial_Check(Trigger *trigger, Relation fk_rel, Relation pk_rel)
14361440
*/
14371441
if (!has_bypassrls_privilege(GetUserId()) &&
14381442
((pk_rel->rd_rel->relrowsecurity &&
1439-
!object_ownercheck(RelationRelationId, pkrte->relid, GetUserId())) ||
1443+
!object_ownercheck(RelationRelationId, RelationGetRelid(pk_rel),
1444+
GetUserId())) ||
14401445
(fk_rel->rd_rel->relrowsecurity &&
1441-
!object_ownercheck(RelationRelationId, fkrte->relid, GetUserId()))))
1446+
!object_ownercheck(RelationRelationId, RelationGetRelid(fk_rel),
1447+
GetUserId()))))
14421448
return false;
14431449

14441450
/*----------

0 commit comments

Comments
 (0)