Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit64f77c6

Browse files
committed
Fix security checks in selectivity estimation functions.
Commite2d4ef8 (the fix forCVE-2017-7484) added security checksto the selectivity estimation functions to prevent them from runninguser-supplied operators on data obtained from pg_statistic if the userlacks privileges to select from the underlying table. In casesinvolving inheritance/partitioning, those checks were originallyperformed against the child RTE (which for plain inheritance mightactually refer to the parent table). Commit553d2ec then extendedthat to also check the parent RTE, allowing access if the user hadpermissions on either the parent or the child. It turns out, however,that doing any checks using the child RTE is incorrect, sincesecurityQuals is set to NULL when creating an RTE for an inheritancechild (whether it refers to the parent table or the child table), andtherefore such checks do not correctly account for any RLS policies orsecurity barrier views. Therefore, do the security checks using onlythe parent RTE. This is consistent with how RLS policies are applied,and the executor's ACL checks, both of which use only the parenttable's permissions/policies. Similar checks are performed in theextended stats code, so update that in the same way, centralizing allthe checks in a new function.In addition, note that these checks by themselves are insufficient toensure that the user has access to the table's data because, in aquery that goes via a view, they only check that the view owner haspermissions on the underlying table, not that the current user haspermissions on the view itself. In the selectivity estimationfunctions, there is no easy way to navigate from underlying tables toviews, so add permissions checks for all views mentioned in the queryto the planner startup code. If the user lacks permissions on a view,a permissions error will now be reported at planner-startup, and theselectivity estimation functions will not be run.Checking view permissions at planner-startup in this way is a littleugly, since the same checks will be repeated at executor-startup.Longer-term, it might be better to move all the permissions checksfrom the executor to the planner so that permissions errors can bereported sooner, instead of creating a plan that won't ever be run.However, such a change seems too far-reaching to be back-patched.Back-patch to all supported versions. In v13, there is the addedcomplication that UPDATEs and DELETEs on inherited target tables areplanned using inheritance_planner(), which plans each inheritancechild table separately, so that the selectivity estimation functionsdo not know that they are dealing with a child table accessed via itsparent. Handle that by checking access permissions on the top parenttable at planner-startup, in the same way as we do for views. AnysecurityQuals on the top parent table are moved down to the childtables by inheritance_planner(), so they continue to be checked by theselectivity estimation functions.Author: Dean Rasheed <dean.a.rasheed@gmail.com>Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>Reviewed-by: Noah Misch <noah@leadboat.com>Backpatch-through: 13Security:CVE-2025-8713
1 parent0d2734e commit64f77c6

File tree

12 files changed

+596
-304
lines changed

12 files changed

+596
-304
lines changed

‎src/backend/executor/execMain.c‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,6 @@ static void ExecutePlan(QueryDesc *queryDesc,
8484
uint64numberTuples,
8585
ScanDirectiondirection,
8686
DestReceiver*dest);
87-
staticboolExecCheckOneRelPerms(RTEPermissionInfo*perminfo);
8887
staticboolExecCheckPermissionsModified(OidrelOid,Oiduserid,
8988
Bitmapset*modifiedCols,
9089
AclModerequiredPerms);
@@ -643,7 +642,7 @@ ExecCheckPermissions(List *rangeTable, List *rteperminfos,
643642
* ExecCheckOneRelPerms
644643
*Check access permissions for a single relation.
645644
*/
646-
staticbool
645+
bool
647646
ExecCheckOneRelPerms(RTEPermissionInfo*perminfo)
648647
{
649648
AclModerequiredPerms;

‎src/backend/optimizer/plan/planner.c‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@
5858
#include"parser/parsetree.h"
5959
#include"partitioning/partdesc.h"
6060
#include"rewrite/rewriteManip.h"
61+
#include"utils/acl.h"
6162
#include"utils/backend_status.h"
6263
#include"utils/lsyscache.h"
6364
#include"utils/rel.h"
@@ -848,6 +849,38 @@ subquery_planner(PlannerGlobal *glob, Query *parse, PlannerInfo *parent_root,
848849
bms_make_singleton(parse->resultRelation);
849850
}
850851

852+
/*
853+
* This would be a convenient time to check access permissions for all
854+
* relations mentioned in the query, since it would be better to fail now,
855+
* before doing any detailed planning. However, for historical reasons,
856+
* we leave this to be done at executor startup.
857+
*
858+
* Note, however, that we do need to check access permissions for any view
859+
* relations mentioned in the query, in order to prevent information being
860+
* leaked by selectivity estimation functions, which only check view owner
861+
* permissions on underlying tables (see all_rows_selectable() and its
862+
* callers). This is a little ugly, because it means that access
863+
* permissions for views will be checked twice, which is another reason
864+
* why it would be better to do all the ACL checks here.
865+
*/
866+
foreach(l,parse->rtable)
867+
{
868+
RangeTblEntry*rte=lfirst_node(RangeTblEntry,l);
869+
870+
if (rte->perminfoindex!=0&&
871+
rte->relkind==RELKIND_VIEW)
872+
{
873+
RTEPermissionInfo*perminfo;
874+
boolresult;
875+
876+
perminfo=getRTEPermissionInfo(parse->rteperminfos,rte);
877+
result=ExecCheckOneRelPerms(perminfo);
878+
if (!result)
879+
aclcheck_error(ACLCHECK_NO_PRIV,OBJECT_VIEW,
880+
get_rel_name(perminfo->relid));
881+
}
882+
}
883+
851884
/*
852885
* Preprocess RowMark information. We need to do this after subquery
853886
* pullup, so that all base relations are present.

‎src/backend/statistics/extended_stats.c‎

Lines changed: 44 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -1317,14 +1317,17 @@ choose_best_statistics(List *stats, char requiredkind, bool inh,
13171317
*so we can't cope with system columns.
13181318
* *exprs: input/output parameter collecting primitive subclauses within
13191319
*the clause tree
1320+
* *leakproof: input/output parameter recording the leakproofness of the
1321+
*clause tree. This should be true initially, and will be set to false
1322+
*if any operator function used in an OpExpr is not leakproof.
13201323
*
13211324
* Returns false if there is something we definitively can't handle.
13221325
* On true return, we can proceed to match the *exprs against statistics.
13231326
*/
13241327
staticbool
13251328
statext_is_compatible_clause_internal(PlannerInfo*root,Node*clause,
13261329
Indexrelid,Bitmapset**attnums,
1327-
List**exprs)
1330+
List**exprs,bool*leakproof)
13281331
{
13291332
/* Look inside any binary-compatible relabeling (as in examine_variable) */
13301333
if (IsA(clause,RelabelType))
@@ -1359,7 +1362,6 @@ statext_is_compatible_clause_internal(PlannerInfo *root, Node *clause,
13591362
/* (Var/Expr op Const) or (Const op Var/Expr) */
13601363
if (is_opclause(clause))
13611364
{
1362-
RangeTblEntry*rte=root->simple_rte_array[relid];
13631365
OpExpr*expr= (OpExpr*)clause;
13641366
Node*clause_expr;
13651367

@@ -1394,24 +1396,15 @@ statext_is_compatible_clause_internal(PlannerInfo *root, Node *clause,
13941396
return false;
13951397
}
13961398

1397-
/*
1398-
* If there are any securityQuals on the RTE from security barrier
1399-
* views or RLS policies, then the user may not have access to all the
1400-
* table's data, and we must check that the operator is leakproof.
1401-
*
1402-
* If the operator is leaky, then we must ignore this clause for the
1403-
* purposes of estimating with MCV lists, otherwise the operator might
1404-
* reveal values from the MCV list that the user doesn't have
1405-
* permission to see.
1406-
*/
1407-
if (rte->securityQuals!=NIL&&
1408-
!get_func_leakproof(get_opcode(expr->opno)))
1409-
return false;
1399+
/* Check if the operator is leakproof */
1400+
if (*leakproof)
1401+
*leakproof=get_func_leakproof(get_opcode(expr->opno));
14101402

14111403
/* Check (Var op Const) or (Const op Var) clauses by recursing. */
14121404
if (IsA(clause_expr,Var))
14131405
returnstatext_is_compatible_clause_internal(root,clause_expr,
1414-
relid,attnums,exprs);
1406+
relid,attnums,
1407+
exprs,leakproof);
14151408

14161409
/* Otherwise we have (Expr op Const) or (Const op Expr). */
14171410
*exprs=lappend(*exprs,clause_expr);
@@ -1421,7 +1414,6 @@ statext_is_compatible_clause_internal(PlannerInfo *root, Node *clause,
14211414
/* Var/Expr IN Array */
14221415
if (IsA(clause,ScalarArrayOpExpr))
14231416
{
1424-
RangeTblEntry*rte=root->simple_rte_array[relid];
14251417
ScalarArrayOpExpr*expr= (ScalarArrayOpExpr*)clause;
14261418
Node*clause_expr;
14271419
boolexpronleft;
@@ -1461,24 +1453,15 @@ statext_is_compatible_clause_internal(PlannerInfo *root, Node *clause,
14611453
return false;
14621454
}
14631455

1464-
/*
1465-
* If there are any securityQuals on the RTE from security barrier
1466-
* views or RLS policies, then the user may not have access to all the
1467-
* table's data, and we must check that the operator is leakproof.
1468-
*
1469-
* If the operator is leaky, then we must ignore this clause for the
1470-
* purposes of estimating with MCV lists, otherwise the operator might
1471-
* reveal values from the MCV list that the user doesn't have
1472-
* permission to see.
1473-
*/
1474-
if (rte->securityQuals!=NIL&&
1475-
!get_func_leakproof(get_opcode(expr->opno)))
1476-
return false;
1456+
/* Check if the operator is leakproof */
1457+
if (*leakproof)
1458+
*leakproof=get_func_leakproof(get_opcode(expr->opno));
14771459

14781460
/* Check Var IN Array clauses by recursing. */
14791461
if (IsA(clause_expr,Var))
14801462
returnstatext_is_compatible_clause_internal(root,clause_expr,
1481-
relid,attnums,exprs);
1463+
relid,attnums,
1464+
exprs,leakproof);
14821465

14831466
/* Otherwise we have Expr IN Array. */
14841467
*exprs=lappend(*exprs,clause_expr);
@@ -1515,7 +1498,8 @@ statext_is_compatible_clause_internal(PlannerInfo *root, Node *clause,
15151498
*/
15161499
if (!statext_is_compatible_clause_internal(root,
15171500
(Node*)lfirst(lc),
1518-
relid,attnums,exprs))
1501+
relid,attnums,exprs,
1502+
leakproof))
15191503
return false;
15201504
}
15211505

@@ -1529,8 +1513,10 @@ statext_is_compatible_clause_internal(PlannerInfo *root, Node *clause,
15291513

15301514
/* Check Var IS NULL clauses by recursing. */
15311515
if (IsA(nt->arg,Var))
1532-
returnstatext_is_compatible_clause_internal(root, (Node*) (nt->arg),
1533-
relid,attnums,exprs);
1516+
returnstatext_is_compatible_clause_internal(root,
1517+
(Node*) (nt->arg),
1518+
relid,attnums,
1519+
exprs,leakproof);
15341520

15351521
/* Otherwise we have Expr IS NULL. */
15361522
*exprs=lappend(*exprs,nt->arg);
@@ -1569,11 +1555,9 @@ static bool
15691555
statext_is_compatible_clause(PlannerInfo*root,Node*clause,Indexrelid,
15701556
Bitmapset**attnums,List**exprs)
15711557
{
1572-
RangeTblEntry*rte=root->simple_rte_array[relid];
1573-
RelOptInfo*rel=root->simple_rel_array[relid];
15741558
RestrictInfo*rinfo;
15751559
intclause_relid;
1576-
Oiduserid;
1560+
boolleakproof;
15771561

15781562
/*
15791563
* Special-case handling for bare BoolExpr AND clauses, because the
@@ -1613,18 +1597,31 @@ statext_is_compatible_clause(PlannerInfo *root, Node *clause, Index relid,
16131597
clause_relid!=relid)
16141598
return false;
16151599

1616-
/* Check the clause and determine what attributes it references. */
1600+
/*
1601+
* Check the clause, determine what attributes it references, and whether
1602+
* it includes any non-leakproof operators.
1603+
*/
1604+
leakproof= true;
16171605
if (!statext_is_compatible_clause_internal(root, (Node*)rinfo->clause,
1618-
relid,attnums,exprs))
1606+
relid,attnums,exprs,
1607+
&leakproof))
16191608
return false;
16201609

16211610
/*
1622-
* Check that the user has permission to read all required attributes.
1611+
* If the clause includes any non-leakproof operators, check that the user
1612+
* has permission to read all required attributes, otherwise the operators
1613+
* might reveal values from the MCV list that the user doesn't have
1614+
* permission to see. We require all rows to be selectable --- there must
1615+
* be no securityQuals from security barrier views or RLS policies. See
1616+
* similar code in examine_variable(), examine_simple_variable(), and
1617+
* statistic_proc_security_check().
1618+
*
1619+
* Note that for an inheritance child, the permission checks are performed
1620+
* on the inheritance root parent, and whole-table select privilege on the
1621+
* parent doesn't guarantee that the user could read all columns of the
1622+
* child. Therefore we must check all referenced columns.
16231623
*/
1624-
userid=OidIsValid(rel->userid) ?rel->userid :GetUserId();
1625-
1626-
/* Table-level SELECT privilege is sufficient for all columns */
1627-
if (pg_class_aclcheck(rte->relid,userid,ACL_SELECT)!=ACLCHECK_OK)
1624+
if (!leakproof)
16281625
{
16291626
Bitmapset*clause_attnums=NULL;
16301627
intattnum=-1;
@@ -1649,26 +1646,9 @@ statext_is_compatible_clause(PlannerInfo *root, Node *clause, Index relid,
16491646
if (*exprs!=NIL)
16501647
pull_varattnos((Node*)*exprs,relid,&clause_attnums);
16511648

1652-
attnum=-1;
1653-
while ((attnum=bms_next_member(clause_attnums,attnum)) >=0)
1654-
{
1655-
/* Undo the offset */
1656-
AttrNumberattno=attnum+FirstLowInvalidHeapAttributeNumber;
1657-
1658-
if (attno==InvalidAttrNumber)
1659-
{
1660-
/* Whole-row reference, so must have access to all columns */
1661-
if (pg_attribute_aclcheck_all(rte->relid,userid,ACL_SELECT,
1662-
ACLMASK_ALL)!=ACLCHECK_OK)
1663-
return false;
1664-
}
1665-
else
1666-
{
1667-
if (pg_attribute_aclcheck(rte->relid,attno,userid,
1668-
ACL_SELECT)!=ACLCHECK_OK)
1669-
return false;
1670-
}
1671-
}
1649+
/* Must have permission to read all rows from these columns */
1650+
if (!all_rows_selectable(root,relid,clause_attnums))
1651+
return false;
16721652
}
16731653

16741654
/* If we reach here, the clause is OK */

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp