On Fri, May 10, 2019 at 10:19:44AM +0100, Dean Rasheed wrote:
While working on 1aebfbea83c, I noticed that the new multivariate MCV
stats feature suffers from the same problem, and also the original
problems that were fixed in e2d4ef8de8 and earlier --- namely that a
user can see values in the MCV lists that they shouldn't see (values
from tables that they don't have privileges on).

I think there are 2 separate issues here:

1). The table pg_statistic_ext is accessible to anyone, so any user
can see the MCV lists of any table. I think we should give this the
same treatment as pg_statistic, and hide it behind a security barrier
view, revoking public access from the table.

2). The multivariate MCV stats planner code can be made to invoke
user-defined operators, so a user can create a leaky operator and use
it to reveal data values from the MCV lists even if they have no
permissions on the table.

Attached is a draft patch to fix (2), which hooks into
statext_is_compatible_clause().


I think that patch is good.

I haven't thought much about (1). There are some questions about what
exactly the view should look like. Probably it should translate table
oids to names, like pg_stats does, but should it also translate column
indexes to names? That could get fiddly. Should it unpack MCV items?


Yeah. I suggest we add a simple pg_stats_ext view, similar to pg_stats.
It would:

(1) translate the schema / relation / attribute names

 I don't see why translating column indexes to names would be fiddly.
 It's a matter of simple unnest + join, no? Or what issues you see?

(2) include values for ndistinct / dependencies, if built

 Those don't include any actual values, so this should be OK. You might
 make the argument that even this does leak a bit of information (e.g.
 when you can see values in one column, and you know there's a strong
 functional dependence, it tells you something about the other column
 which you may not see). But I think we kinda already leak information
 about that through estimates, so maybe that's not an issue.

(3) include MCV list only when user has access to all columns

 Essentially, if the user is missing 'select' privilege on at least one
 of the columns, there'll be NULL. Otherwise the MCV value.

The attached patch adds pg_stats_ext doing this. I don't claim it's the
best possible query backing the view, so any improvements are welcome.


I've been thinking we might somehow filter the statistics values, e.g. by
not showing values for attributes the user has no 'select' privilege on,
but that seems like a can of worms. It'd lead to MCV items that can't be
distinguished because the only difference was the removed attribute, and
so on. So I propose we simply show/hide the whole MCV list.

Likewise, I don't think it makes sense to expand the MCV list in this
view - that works for the single-dimensional case, because then the
list is expanded into two arrays (values + frequencies), which are easy
to process further. But for multivariate MCV lists that's much more
complex - we don't know how many attributes are there, for example.

So I suggest we just show the pg_mcv_list value as is, and leave it up
to the user to call the pg_mcv_list_items() function if needed.

This will also work for histograms, where expanding the value in the
pg_stats_ext would be even trickier.


--
Tomas Vondra                  http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
diff --git a/src/backend/catalog/system_views.sql 
b/src/backend/catalog/system_views.sql
index 566100d6df..b1302cfa5d 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -253,6 +253,26 @@ CREATE VIEW pg_stats WITH (security_barrier) AS
 
 REVOKE ALL on pg_statistic FROM public;
 
+CREATE VIEW pg_stats_ext WITH (security_barrier) AS
+    SELECT
+        nspname AS schemaname,
+        relname AS tablename,
+        (SELECT array_agg(attname) FROM unnest(stxkeys) k
+           JOIN pg_attribute a ON (a.attnum = k)
+          WHERE attrelid = stxrelid) AS attnames,
+        stxkind,
+        stxndistinct,
+        stxdependencies,
+        (CASE WHEN EXISTS (SELECT 1 FROM unnest(stxkeys) k
+           JOIN pg_attribute a ON (a.attnum = k)
+          WHERE attrelid = stxrelid AND NOT has_column_privilege(c.oid, 
a.attnum, 'select')) THEN NULL
+         ELSE stxmcv END) stxmcv
+    FROM pg_statistic_ext s JOIN pg_class c ON (c.oid = s.stxrelid)
+         LEFT JOIN pg_namespace n ON (n.oid = c.relnamespace)
+    WHERE (c.relrowsecurity = false OR NOT row_security_active(c.oid));
+
+REVOKE ALL on pg_statistic_ext FROM public;
+
 CREATE VIEW pg_publication_tables AS
     SELECT
         P.pubname AS pubname,

Reply via email to