Allow access to child table statistics if user can read parent table.

The fix for CVE-2017-7484 disallowed use of pg_statistic data for
planning purposes if the user would not be able to select the associated
column and a non-leakproof function is to be applied to the statistics
values.  That turns out to disable use of pg_statistic data in some
common cases involving inheritance/partitioning, where the user does
have permission to select from the parent table that was actually named
in the query, but not from a child table whose stats are needed.  Since,
in non-corner cases, the user *can* select the child table's data via
the parent, this restriction is not actually useful from a security
standpoint.  Improve the logic so that we also check the permissions of
the originally-named table, and allow access if select permission exists
for that.

When checking access to stats for a simple child column, we can map
the child column number back to the parent, and perform this test
exactly (including not allowing access if the child column isn't
exposed by the parent).  For expression indexes, the current logic
just insists on whole-table select access, and this patch allows
access if the user can select the whole parent table.  In principle,
if the child table has extra columns, this might allow access to
stats on columns the user can't read.  In practice, it's unlikely
that the planner is going to do any stats calculations involving
expressions that are not visible to the query, so we'll ignore that
fine point for now.  Perhaps someday we'll improve that logic to
detect exactly which columns are used by an expression index ...
but today is not that day.

Back-patch to v11.  The issue was created in 9.2 and up by the
CVE-2017-7484 fix, but this patch depends on the append_rel_array[]
planner data structure which only exists in v11 and up.  In
practice the issue is most urgent with partitioned tables, so
fixing v11 and later should satisfy much of the practical need.

Dilip Kumar and Amit Langote, with some kibitzing by me

Discussion: https://postgr.es/m/3876.1531261875@sss.pgh.pa.us
This commit is contained in:
Tom Lane 2019-11-26 14:41:48 -05:00
parent 12198239c0
commit 553d2ec271
3 changed files with 240 additions and 0 deletions

View file

@ -4613,6 +4613,52 @@ examine_variable(PlannerInfo *root, Node *node, int varRelid,
rte->securityQuals == NIL &&
(pg_class_aclcheck(rte->relid, userid,
ACL_SELECT) == ACLCHECK_OK);
/*
* If the user doesn't have permissions to
* access an inheritance child relation, check
* the permissions of the table actually
* mentioned in the query, since most likely
* the user does have that permission. Note
* that whole-table select privilege on the
* parent doesn't quite guarantee that the
* user could read all columns of the child.
* But in practice it's unlikely that any
* interesting security violation could result
* from allowing access to the expression
* index's stats, so we allow it anyway. See
* similar code in examine_simple_variable()
* for additional comments.
*/
if (!vardata->acl_ok &&
root->append_rel_array != NULL)
{
AppendRelInfo *appinfo;
Index varno = index->rel->relid;
appinfo = root->append_rel_array[varno];
while (appinfo &&
planner_rt_fetch(appinfo->parent_relid,
root)->rtekind == RTE_RELATION)
{
varno = appinfo->parent_relid;
appinfo = root->append_rel_array[varno];
}
if (varno != index->rel->relid)
{
/* Repeat access check on this rel */
rte = planner_rt_fetch(varno, root);
Assert(rte->rtekind == RTE_RELATION);
userid = rte->checkAsUser ? rte->checkAsUser : GetUserId();
vardata->acl_ok =
rte->securityQuals == NIL &&
(pg_class_aclcheck(rte->relid,
userid,
ACL_SELECT) == ACLCHECK_OK);
}
}
}
else
{
@ -4690,6 +4736,88 @@ examine_simple_variable(PlannerInfo *root, Var *var,
ACL_SELECT) == ACLCHECK_OK) ||
(pg_attribute_aclcheck(rte->relid, var->varattno, userid,
ACL_SELECT) == ACLCHECK_OK));
/*
* If the user doesn't have permissions to access an inheritance
* child relation or specifically this attribute, check the
* permissions of the table/column actually mentioned in the
* query, since most likely the user does have that permission
* (else the query will fail at runtime), and if the user can read
* the column there then he can get the values of the child table
* too. To do that, we must find out which of the root parent's
* attributes the child relation's attribute corresponds to.
*/
if (!vardata->acl_ok && var->varattno > 0 &&
root->append_rel_array != NULL)
{
AppendRelInfo *appinfo;
Index varno = var->varno;
int varattno = var->varattno;
bool found = false;
appinfo = root->append_rel_array[varno];
/*
* Partitions are mapped to their immediate parent, not the
* root parent, so must be ready to walk up multiple
* AppendRelInfos. But stop if we hit a parent that is not
* RTE_RELATION --- that's a flattened UNION ALL subquery, not
* an inheritance parent.
*/
while (appinfo &&
planner_rt_fetch(appinfo->parent_relid,
root)->rtekind == RTE_RELATION)
{
int parent_varattno;
ListCell *l;
parent_varattno = 1;
found = false;
foreach(l, appinfo->translated_vars)
{
Var *childvar = lfirst_node(Var, l);
/* Ignore dropped attributes of the parent. */
if (childvar != NULL &&
varattno == childvar->varattno)
{
found = true;
break;
}
parent_varattno++;
}
if (!found)
break;
varno = appinfo->parent_relid;
varattno = parent_varattno;
/* If the parent is itself a child, continue up. */
appinfo = root->append_rel_array[varno];
}
/*
* In rare cases, the Var may be local to the child table, in
* which case, we've got to live with having no access to this
* column's stats.
*/
if (!found)
return;
/* Repeat the access check on this parent rel & column */
rte = planner_rt_fetch(varno, root);
Assert(rte->rtekind == RTE_RELATION);
userid = rte->checkAsUser ? rte->checkAsUser : GetUserId();
vardata->acl_ok =
rte->securityQuals == NIL &&
((pg_class_aclcheck(rte->relid, userid,
ACL_SELECT) == ACLCHECK_OK) ||
(pg_attribute_aclcheck(rte->relid, varattno, userid,
ACL_SELECT) == ACLCHECK_OK));
}
}
else
{

View file

@ -2335,3 +2335,77 @@ explain (costs off) select * from range_parted order by a desc,b desc,c desc;
(3 rows)
drop table range_parted;
-- Check that we allow access to a child table's statistics when the user
-- has permissions only for the parent table.
create table permtest_parent (a int, b text, c text) partition by list (a);
create table permtest_child (b text, c text, a int) partition by list (b);
create table permtest_grandchild (c text, b text, a int);
alter table permtest_child attach partition permtest_grandchild for values in ('a');
alter table permtest_parent attach partition permtest_child for values in (1);
create index on permtest_parent (left(c, 3));
insert into permtest_parent
select 1, 'a', left(md5(i::text), 5) from generate_series(0, 100) i;
analyze permtest_parent;
create role regress_no_child_access;
revoke all on permtest_grandchild from regress_no_child_access;
grant select on permtest_parent to regress_no_child_access;
set session authorization regress_no_child_access;
-- without stats access, these queries would produce hash join plans:
explain (costs off)
select * from permtest_parent p1 inner join permtest_parent p2
on p1.a = p2.a and p1.c ~ 'a1$';
QUERY PLAN
------------------------------------------
Nested Loop
Join Filter: (p1.a = p2.a)
-> Seq Scan on permtest_grandchild p1
Filter: (c ~ 'a1$'::text)
-> Seq Scan on permtest_grandchild p2
(5 rows)
explain (costs off)
select * from permtest_parent p1 inner join permtest_parent p2
on p1.a = p2.a and left(p1.c, 3) ~ 'a1$';
QUERY PLAN
----------------------------------------------
Nested Loop
Join Filter: (p1.a = p2.a)
-> Seq Scan on permtest_grandchild p1
Filter: ("left"(c, 3) ~ 'a1$'::text)
-> Seq Scan on permtest_grandchild p2
(5 rows)
reset session authorization;
revoke all on permtest_parent from regress_no_child_access;
grant select(a,c) on permtest_parent to regress_no_child_access;
set session authorization regress_no_child_access;
explain (costs off)
select p2.a, p1.c from permtest_parent p1 inner join permtest_parent p2
on p1.a = p2.a and p1.c ~ 'a1$';
QUERY PLAN
------------------------------------------
Nested Loop
Join Filter: (p1.a = p2.a)
-> Seq Scan on permtest_grandchild p1
Filter: (c ~ 'a1$'::text)
-> Seq Scan on permtest_grandchild p2
(5 rows)
-- we will not have access to the expression index's stats here:
explain (costs off)
select p2.a, p1.c from permtest_parent p1 inner join permtest_parent p2
on p1.a = p2.a and left(p1.c, 3) ~ 'a1$';
QUERY PLAN
----------------------------------------------------
Hash Join
Hash Cond: (p2.a = p1.a)
-> Seq Scan on permtest_grandchild p2
-> Hash
-> Seq Scan on permtest_grandchild p1
Filter: ("left"(c, 3) ~ 'a1$'::text)
(6 rows)
reset session authorization;
revoke all on permtest_parent from regress_no_child_access;
drop role regress_no_child_access;
drop table permtest_parent;

View file

@ -845,3 +845,41 @@ explain (costs off) select * from range_parted order by a,b,c;
explain (costs off) select * from range_parted order by a desc,b desc,c desc;
drop table range_parted;
-- Check that we allow access to a child table's statistics when the user
-- has permissions only for the parent table.
create table permtest_parent (a int, b text, c text) partition by list (a);
create table permtest_child (b text, c text, a int) partition by list (b);
create table permtest_grandchild (c text, b text, a int);
alter table permtest_child attach partition permtest_grandchild for values in ('a');
alter table permtest_parent attach partition permtest_child for values in (1);
create index on permtest_parent (left(c, 3));
insert into permtest_parent
select 1, 'a', left(md5(i::text), 5) from generate_series(0, 100) i;
analyze permtest_parent;
create role regress_no_child_access;
revoke all on permtest_grandchild from regress_no_child_access;
grant select on permtest_parent to regress_no_child_access;
set session authorization regress_no_child_access;
-- without stats access, these queries would produce hash join plans:
explain (costs off)
select * from permtest_parent p1 inner join permtest_parent p2
on p1.a = p2.a and p1.c ~ 'a1$';
explain (costs off)
select * from permtest_parent p1 inner join permtest_parent p2
on p1.a = p2.a and left(p1.c, 3) ~ 'a1$';
reset session authorization;
revoke all on permtest_parent from regress_no_child_access;
grant select(a,c) on permtest_parent to regress_no_child_access;
set session authorization regress_no_child_access;
explain (costs off)
select p2.a, p1.c from permtest_parent p1 inner join permtest_parent p2
on p1.a = p2.a and p1.c ~ 'a1$';
-- we will not have access to the expression index's stats here:
explain (costs off)
select p2.a, p1.c from permtest_parent p1 inner join permtest_parent p2
on p1.a = p2.a and left(p1.c, 3) ~ 'a1$';
reset session authorization;
revoke all on permtest_parent from regress_no_child_access;
drop role regress_no_child_access;
drop table permtest_parent;