Re: Sloppy thinking about leakproof properties of opclass co-members

On Wed, Sep 24, 2014 at 5:34 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> It strikes me that there's a significant gap in the whole "leakproof
> function" business, namely that no consideration has been given to
> planner-driven transformations of queries. As an example, if we
> have "a = b" and "b = c", the planner may generate and apply "a = c"
> instead of one or both of those clauses. If a, b, c are not all the
> same type, "a = c" might involve an operator that's not either of the
> original ones. And it's possible that that operator is not leakproof
> where the original ones are. This could easily result in introducing
> non-leakproof operations into a secure subquery after pushdown of a
> clause that was marked secure.
>
> Another example is that in attempting to make implication or refutation
> proofs involving operator clauses, the planner feels free to apply other
> members of the operator's btree opclass (if it's in one). I've not
> bothered to try to create a working exploit, but I'm pretty sure that
> this could result in a non-leakproof function being applied during
> planning of a supposedly secure subquery. It might be that that couldn't
> leak anything worse than constant values within the query tree, but
> perhaps it could leak data values from a protected table's pg_statistic
> entries.
>
> ISTM that the most appropriate solution here is to insist that all or none
> of the members of an operator class be marked leakproof. (Possibly we
> could restrict that to btree opclasses, but I'm not sure any exception is
> needed for other index types.) I looked for existing violations of this
> precept, and unfortunately found a *lot*. For example, texteq is marked
> leakproof but its fellow text comparison operators aren't. Is that really
> sane?

Not really. Fortunately, AFAICT, most if not all of these are in the
good direction: there are some things not marked leakproof that can be
so marked. The reverse direction would be a hard-to-fix security
hole. I think at some point somebody went through and tried to mark
all of the same-type equality operators as leakproof, and it seems
like that got expanded somewhat without fully rationalizing what we
had in pg_proc... mostly because I think nobody had a clear idea how
to do that.

I think your proposal here is a good one. Heikki proposed treating
opclass functions as leakproof *in lieu of* adding a flag, but that
didn't seem good because we wanted to allow for the possibility of
cases where that wasn't true, and ensure individual scrutiny of each
case. Your idea of making sure the flag is set consistently
throughout the opclass (opfamily?) is similar in spirit, but better in
detail.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company