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

On 26 September 2014 15:48, Stephen Frost <sfrost@snowman.net> wrote:
> * Robert Haas (robertmhaas@gmail.com) wrote:
>> On Wed, Sep 24, 2014 at 5:34 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> > 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.
>
> We'll need to investigate and see if there are any which *aren't* safe
> and probably fix those to be safe rather than trying to deal with this
> risk in some other way.  In other words, I hope it's really "all of
> these" rather than just most.  In general, I've been hoping that we have
> more leakproof functions than not as, while it's non-trivial to write
> them and ensure they're correct, it's better for us to take that on than
> to ask our users to do so (or have them get some set that someone else
> wrote off of a website or even the extension network).  We can't cover
> everything, of course, but hopefully we'll cover all reasonable use
> cases for types we ship.
>

Looking at other functions, ISTM that there's an entire class of
functions that can trivially be marked leakproof, and that's no-arg
functions which can't possibly leak. There are quite a few no-arg
builtin functions and none of them are currently marked leakproof.

Rather than (or perhaps as well as) marking all these leakproof,
perhaps we should teach contain_leaky_functions() to automatically
treat any no-arg function as leakproof, so that we allow user-defined
functions too. Taking that one step further, perhaps what it should
really be looking for is Vars in the argument list of a leaky
function, i.e., contain_leaked_vars() rather than
contain_leaky_functions().

Regards,
Dean