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

Commitd7f8d26

Browse files
committed
Add security checks to the multivariate MCV estimation code.
The multivariate MCV estimation code may run user-defined operators onthe values in the MCV list, which means that those operators maypotentially leak the values from the MCV list. Guard against leakingdata to unprivileged users by checking that the user has SELECTprivileges on the table or all of the columns referred to by thestatistics.Additionally, if there are any securityQuals on the RTE (either due toRLS policies on the table, or accessing the table via a securitybarrier view), not all rows may be visible to the current user, evenif they have table or column privileges. Thus we further insist thatthe operator be leakproof in this case.Dean Rasheed, reviewed by Tomas Vondra.Discussion:https://postgr.es/m/CAEZATCUhT9rt7Ui=Vdx4N==VV5XOK5dsXfnGgVOz_JhAicB=ZA@mail.gmail.com
1 parent89ff7c0 commitd7f8d26

File tree

3 files changed

+186
-8
lines changed

3 files changed

+186
-8
lines changed

‎src/backend/statistics/extended_stats.c

Lines changed: 66 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
#include"catalog/pg_collation.h"
2525
#include"catalog/pg_statistic_ext.h"
2626
#include"catalog/pg_statistic_ext_data.h"
27+
#include"miscadmin.h"
2728
#include"nodes/nodeFuncs.h"
2829
#include"optimizer/clauses.h"
2930
#include"optimizer/optimizer.h"
@@ -760,7 +761,8 @@ choose_best_statistics(List *stats, Bitmapset *attnums, char requiredkind)
760761
* attribute numbers from all compatible clauses (recursively).
761762
*/
762763
staticbool
763-
statext_is_compatible_clause_internal(Node*clause,Indexrelid,Bitmapset**attnums)
764+
statext_is_compatible_clause_internal(PlannerInfo*root,Node*clause,
765+
Indexrelid,Bitmapset**attnums)
764766
{
765767
/* Look inside any binary-compatible relabeling (as in examine_variable) */
766768
if (IsA(clause,RelabelType))
@@ -791,6 +793,7 @@ statext_is_compatible_clause_internal(Node *clause, Index relid, Bitmapset **att
791793
/* (Var op Const) or (Const op Var) */
792794
if (is_opclause(clause))
793795
{
796+
RangeTblEntry*rte=root->simple_rte_array[relid];
794797
OpExpr*expr= (OpExpr*)clause;
795798
Var*var;
796799
boolvaronleft= true;
@@ -833,9 +836,24 @@ statext_is_compatible_clause_internal(Node *clause, Index relid, Bitmapset **att
833836
return false;
834837
}
835838

839+
/*
840+
* If there are any securityQuals on the RTE from security barrier
841+
* views or RLS policies, then the user may not have access to all the
842+
* table's data, and we must check that the operator is leak-proof.
843+
*
844+
* If the operator is leaky, then we must ignore this clause for the
845+
* purposes of estimating with MCV lists, otherwise the operator might
846+
* reveal values from the MCV list that the user doesn't have
847+
* permission to see.
848+
*/
849+
if (rte->securityQuals!=NIL&&
850+
!get_func_leakproof(get_opcode(expr->opno)))
851+
return false;
852+
836853
var= (varonleft) ?linitial(expr->args) :lsecond(expr->args);
837854

838-
returnstatext_is_compatible_clause_internal((Node*)var,relid,attnums);
855+
returnstatext_is_compatible_clause_internal(root, (Node*)var,
856+
relid,attnums);
839857
}
840858

841859
/* AND/OR/NOT clause */
@@ -866,7 +884,8 @@ statext_is_compatible_clause_internal(Node *clause, Index relid, Bitmapset **att
866884
* Had we found incompatible clause in the arguments, treat the
867885
* whole clause as incompatible.
868886
*/
869-
if (!statext_is_compatible_clause_internal((Node*)lfirst(lc),
887+
if (!statext_is_compatible_clause_internal(root,
888+
(Node*)lfirst(lc),
870889
relid,attnums))
871890
return false;
872891
}
@@ -886,7 +905,8 @@ statext_is_compatible_clause_internal(Node *clause, Index relid, Bitmapset **att
886905
if (!IsA(nt->arg,Var))
887906
return false;
888907

889-
returnstatext_is_compatible_clause_internal((Node*) (nt->arg),relid,attnums);
908+
returnstatext_is_compatible_clause_internal(root, (Node*) (nt->arg),
909+
relid,attnums);
890910
}
891911

892912
return false;
@@ -909,9 +929,12 @@ statext_is_compatible_clause_internal(Node *clause, Index relid, Bitmapset **att
909929
* complex cases, for example (Var op Var).
910930
*/
911931
staticbool
912-
statext_is_compatible_clause(Node*clause,Indexrelid,Bitmapset**attnums)
932+
statext_is_compatible_clause(PlannerInfo*root,Node*clause,Indexrelid,
933+
Bitmapset**attnums)
913934
{
935+
RangeTblEntry*rte=root->simple_rte_array[relid];
914936
RestrictInfo*rinfo= (RestrictInfo*)clause;
937+
Oiduserid;
915938

916939
if (!IsA(rinfo,RestrictInfo))
917940
return false;
@@ -924,8 +947,43 @@ statext_is_compatible_clause(Node *clause, Index relid, Bitmapset **attnums)
924947
if (bms_membership(rinfo->clause_relids)!=BMS_SINGLETON)
925948
return false;
926949

927-
returnstatext_is_compatible_clause_internal((Node*)rinfo->clause,
928-
relid,attnums);
950+
/* Check the clause and determine what attributes it references. */
951+
if (!statext_is_compatible_clause_internal(root, (Node*)rinfo->clause,
952+
relid,attnums))
953+
return false;
954+
955+
/*
956+
* Check that the user has permission to read all these attributes. Use
957+
* checkAsUser if it's set, in case we're accessing the table via a view.
958+
*/
959+
userid=rte->checkAsUser ?rte->checkAsUser :GetUserId();
960+
961+
if (pg_class_aclcheck(rte->relid,userid,ACL_SELECT)!=ACLCHECK_OK)
962+
{
963+
/* Don't have table privilege, must check individual columns */
964+
if (bms_is_member(InvalidAttrNumber,*attnums))
965+
{
966+
/* Have a whole-row reference, must have access to all columns */
967+
if (pg_attribute_aclcheck_all(rte->relid,userid,ACL_SELECT,
968+
ACLMASK_ALL)!=ACLCHECK_OK)
969+
return false;
970+
}
971+
else
972+
{
973+
/* Check the columns referenced by the clause */
974+
intattnum=-1;
975+
976+
while ((attnum=bms_next_member(*attnums,attnum)) >=0)
977+
{
978+
if (pg_attribute_aclcheck(rte->relid,attnum,userid,
979+
ACL_SELECT)!=ACLCHECK_OK)
980+
return false;
981+
}
982+
}
983+
}
984+
985+
/* If we reach here, the clause is OK */
986+
return true;
929987
}
930988

931989
/*
@@ -1027,7 +1085,7 @@ statext_mcv_clauselist_selectivity(PlannerInfo *root, List *clauses, int varReli
10271085
Bitmapset*attnums=NULL;
10281086

10291087
if (!bms_is_member(listidx,*estimatedclauses)&&
1030-
statext_is_compatible_clause(clause,rel->relid,&attnums))
1088+
statext_is_compatible_clause(root,clause,rel->relid,&attnums))
10311089
{
10321090
list_attnums[listidx]=attnums;
10331091
clauses_attnums=bms_add_members(clauses_attnums,attnums);

‎src/test/regress/expected/stats_ext.out

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -696,3 +696,63 @@ SELECT * FROM check_estimated_rows('SELECT * FROM mcv_lists_bool WHERE NOT a AND
696696
1 | 0
697697
(1 row)
698698

699+
-- Permission tests. Users should not be able to see specific data values in
700+
-- the extended statistics, if they lack permission to see those values in
701+
-- the underlying table.
702+
--
703+
-- Currently this is only relevant for MCV stats.
704+
CREATE TABLE priv_test_tbl (
705+
a int,
706+
b int
707+
);
708+
INSERT INTO priv_test_tbl
709+
SELECT mod(i,5), mod(i,10) FROM generate_series(1,100) s(i);
710+
CREATE STATISTICS priv_test_stats (mcv) ON a, b
711+
FROM priv_test_tbl;
712+
ANALYZE priv_test_tbl;
713+
-- User with no access
714+
CREATE USER regress_stats_user1;
715+
SET SESSION AUTHORIZATION regress_stats_user1;
716+
SELECT * FROM priv_test_tbl; -- Permission denied
717+
ERROR: permission denied for table priv_test_tbl
718+
-- Attempt to gain access using a leaky operator
719+
CREATE FUNCTION op_leak(int, int) RETURNS bool
720+
AS 'BEGIN RAISE NOTICE ''op_leak => %, %'', $1, $2; RETURN $1 < $2; END'
721+
LANGUAGE plpgsql;
722+
CREATE OPERATOR <<< (procedure = op_leak, leftarg = int, rightarg = int,
723+
restrict = scalarltsel);
724+
SELECT * FROM priv_test_tbl WHERE a <<< 0 AND b <<< 0; -- Permission denied
725+
ERROR: permission denied for table priv_test_tbl
726+
DELETE FROM priv_test_tbl WHERE a <<< 0 AND b <<< 0; -- Permission denied
727+
ERROR: permission denied for table priv_test_tbl
728+
-- Grant access via a security barrier view, but hide all data
729+
RESET SESSION AUTHORIZATION;
730+
CREATE VIEW priv_test_view WITH (security_barrier=true)
731+
AS SELECT * FROM priv_test_tbl WHERE false;
732+
GRANT SELECT, DELETE ON priv_test_view TO regress_stats_user1;
733+
-- Should now have access via the view, but see nothing and leak nothing
734+
SET SESSION AUTHORIZATION regress_stats_user1;
735+
SELECT * FROM priv_test_view WHERE a <<< 0 AND b <<< 0; -- Should not leak
736+
a | b
737+
---+---
738+
(0 rows)
739+
740+
DELETE FROM priv_test_view WHERE a <<< 0 AND b <<< 0; -- Should not leak
741+
-- Grant table access, but hide all data with RLS
742+
RESET SESSION AUTHORIZATION;
743+
ALTER TABLE priv_test_tbl ENABLE ROW LEVEL SECURITY;
744+
GRANT SELECT, DELETE ON priv_test_tbl TO regress_stats_user1;
745+
-- Should now have direct table access, but see nothing and leak nothing
746+
SET SESSION AUTHORIZATION regress_stats_user1;
747+
SELECT * FROM priv_test_tbl WHERE a <<< 0 AND b <<< 0; -- Should not leak
748+
a | b
749+
---+---
750+
(0 rows)
751+
752+
DELETE FROM priv_test_tbl WHERE a <<< 0 AND b <<< 0; -- Should not leak
753+
-- Tidy up
754+
DROP OPERATOR <<< (int, int);
755+
DROP FUNCTION op_leak(int, int);
756+
RESET SESSION AUTHORIZATION;
757+
DROP VIEW priv_test_view;
758+
DROP TABLE priv_test_tbl;

‎src/test/regress/sql/stats_ext.sql

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -446,3 +446,63 @@ SELECT * FROM check_estimated_rows('SELECT * FROM mcv_lists_bool WHERE NOT a AND
446446
SELECT*FROM check_estimated_rows('SELECT * FROM mcv_lists_bool WHERE NOT a AND NOT b AND c');
447447

448448
SELECT*FROM check_estimated_rows('SELECT * FROM mcv_lists_bool WHERE NOT a AND b AND NOT c');
449+
450+
-- Permission tests. Users should not be able to see specific data values in
451+
-- the extended statistics, if they lack permission to see those values in
452+
-- the underlying table.
453+
--
454+
-- Currently this is only relevant for MCV stats.
455+
CREATETABLEpriv_test_tbl (
456+
aint,
457+
bint
458+
);
459+
460+
INSERT INTO priv_test_tbl
461+
SELECT mod(i,5), mod(i,10)FROM generate_series(1,100) s(i);
462+
463+
CREATE STATISTICS priv_test_stats (mcv)ON a, b
464+
FROM priv_test_tbl;
465+
466+
ANALYZE priv_test_tbl;
467+
468+
-- User with no access
469+
CREATEUSERregress_stats_user1;
470+
SET SESSION AUTHORIZATION regress_stats_user1;
471+
SELECT*FROM priv_test_tbl;-- Permission denied
472+
473+
-- Attempt to gain access using a leaky operator
474+
CREATEFUNCTIONop_leak(int,int) RETURNS bool
475+
AS'BEGIN RAISE NOTICE''op_leak => %, %'', $1, $2; RETURN $1 < $2; END'
476+
LANGUAGE plpgsql;
477+
CREATE OPERATOR<<< (procedure= op_leak, leftarg=int, rightarg=int,
478+
restrict= scalarltsel);
479+
SELECT*FROM priv_test_tblWHERE a<<<0AND b<<<0;-- Permission denied
480+
DELETEFROM priv_test_tblWHERE a<<<0AND b<<<0;-- Permission denied
481+
482+
-- Grant access via a security barrier view, but hide all data
483+
RESET SESSION AUTHORIZATION;
484+
CREATEVIEWpriv_test_view WITH (security_barrier=true)
485+
ASSELECT*FROM priv_test_tblWHERE false;
486+
GRANTSELECT,DELETEON priv_test_view TO regress_stats_user1;
487+
488+
-- Should now have access via the view, but see nothing and leak nothing
489+
SET SESSION AUTHORIZATION regress_stats_user1;
490+
SELECT*FROM priv_test_viewWHERE a<<<0AND b<<<0;-- Should not leak
491+
DELETEFROM priv_test_viewWHERE a<<<0AND b<<<0;-- Should not leak
492+
493+
-- Grant table access, but hide all data with RLS
494+
RESET SESSION AUTHORIZATION;
495+
ALTERTABLE priv_test_tbl ENABLE ROW LEVEL SECURITY;
496+
GRANTSELECT,DELETEON priv_test_tbl TO regress_stats_user1;
497+
498+
-- Should now have direct table access, but see nothing and leak nothing
499+
SET SESSION AUTHORIZATION regress_stats_user1;
500+
SELECT*FROM priv_test_tblWHERE a<<<0AND b<<<0;-- Should not leak
501+
DELETEFROM priv_test_tblWHERE a<<<0AND b<<<0;-- Should not leak
502+
503+
-- Tidy up
504+
DROPOPERATOR<<< (int,int);
505+
DROPFUNCTION op_leak(int,int);
506+
RESET SESSION AUTHORIZATION;
507+
DROPVIEW priv_test_view;
508+
DROPTABLE priv_test_tbl;

0 commit comments

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