Re: [Patch] Invalid permission check in pg_stats for functional indexes

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>
Cc: Pierre Ducroquet <p(dot)psql(at)pinaraf(dot)info>, pgsql-hackers(at)lists(dot)postgresql(dot)org, Kuntal Ghosh <kuntalghosh(dot)2007(at)gmail(dot)com>
Subject: Re: [Patch] Invalid permission check in pg_stats for functional indexes
Date: 2019-12-26 23:46:59
Message-ID: 9243.1577404019@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Awhile back I wrote:
> Actually ... maybe we don't need to change the view definition at all,
> but instead just make has_column_privilege() do something different
> for indexes than it does for other relation types. It's dubious that
> applying that function to an index yields anything meaningful today,
> so we could redefine what it returns without (probably) breaking
> anything. That would at least give us an option to back-patch, too,
> though the end result might be complex enough that we don't care to
> risk it.

In hopes of resurrecting this thread, here's a draft patch that does
it like that (and also fixes row_security_active(), as otherwise this
probably creates a security hole in pg_stats).

It's definitely not commit quality as-is, for several reasons:

* No regression tests.

* I didn't bother to flesh out logic for looking at the individual
column privileges. I'm not sure if that's worth doing. If it is,
we should also look at BuildIndexValueDescription() which is worrying
about largely the same thing, and likewise is punting on the hardest
cases; and selfuncs.c's examine_variable, ditto; and maybe other places.
They should all be able to share one implementation of a check for
whether the user can read all the columns the index depends on.

* There's still the issue of whether any of the other nearby privilege
checking functions need to be synchronized with this, for consistency's
sake. The pg_stats view doesn't care about the others, but I think
it's a bit weird if has_column_privilege works like this but, say,
has_table_privilege doesn't.

More generally, does anyone want to object to the whole concept of
redefining the has_xxx_privilege functions' behavior when applied
to indexes?

regards, tom lane

Attachment Content-Type Size
has-column-privilege-for-indexes-wip.patch text/x-diff 6.1 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message John Naylor 2019-12-26 23:49:46 use CLZ instruction in AllocSetFreeIndex()
Previous Message Maciek Sakrejda 2019-12-26 23:31:16 Re: Duplicate Workers entries in some EXPLAIN plans