Re: Is MinMaxExpr really leakproof?

Stephen Frost <sfrost(at)snowman(dot)net> writes:
> * Tom Lane (tgl(at)sss(dot)pgh(dot)pa(dot)us) wrote:
>> Noah Misch <noah(at)leadboat(dot)com> writes:
>>> Either of those solutions sounds fine. Like last time, I'll vote for explicitly
>>> verifying leakproofness.

>> Yeah, I'm leaning in that direction as well. Other than comparisons
>> involving strings, it's not clear that we'd gain much from insisting
>> on leakproofness in general, and it seems like it might be rather a
>> large burden to put on add-on data types.

> While I'd actually like it if we required leakproofness for what we
> ship, I agree that we shouldn't blindly assume that add-on data types
> are always leakproof and that then requires that we explicitly verify
> it. Perhaps an argument can be made that there are some cases where
> what we ship can't or shouldn't be leakproof for usability but, ideally,
> those would be relatively rare exceptions that don't impact common
> use-cases.

Seems like we have consensus that MinMaxExpr should verify leakproofness
rather than just assume it, so I'll go fix that.

What's your opinion on the question of whether to try to make text_cmp
et al leakproof? I notice that texteq/textne are (correctly) marked
leakproof, so perhaps the usability issue isn't as pressing as I first
thought; but it remains true that there are fairly common cases where
the current marking is going to impede optimization. I also realized
that in the wake of 586b98fdf, we have to remove the leakproofness
marking of name_cmp and name inequality comparisons --- which I did
at d01e75d68, but that's potentially a regression in optimizability
of catalog queries, so it's not very nice.

Also, I believe that Peter's work towards making text equality potentially
collation-sensitive will destroy the excuse for marking texteq/textne
leakproof if we're going to be 100% rigid about that, and that would be
a very big regression.

So I'd like to get to a point where we're comfortable marking these
functions leakproof despite the possibility of corner-case failures.
We could just decide that the existing failure cases in varstr_cmp are
not usefully exploitable for information leakage, or perhaps we could
dumb down the error messages some more to make them even less so.
It'd also be nice to have some articulatable policy that encompasses
a choice like this.

Thoughts?

regards, tom lane