Re: FOR PORTION OF vs. object_aclcheck

On 04.05.26 15:36, Robert Haas wrote:
> Hi,
>
> I noticed today that ExecForPortionOfLeftovers initializes an FmgrInfo
> for forPortionOf->withoutPortionProc without checking whether the user
> has ACL_EXECUTE on that function. Currently, this does not really
> matter, because transformForPortionOfClause only ever sets
> withoutPortionProc to F_RANGE_MINUS_MULTI or F_MULTIRANGE_MINUS_MULTI,
> which are publicly executable anyway, at least by default. However,
> there's a comment that says this:
>
> * XXX: Find a more extensible way to look up the function, permitting
> * user-defined types. An opclass support function doesn't make sense,
> * since there is no index involved. Perhaps a type support function.
>
> So maybe in the future this will be able to call a wider range of
> functions. It's possible that won't really matter from a security
> perspective either since maybe you'd have to be superuser to get a
> function OID of your own choosing into withoutPortionProc. However, I
> think it might be a good idea to add a permission check here anyway,
> just to be on the safe side. I can't see any real downside to
> insisting that the user actually executing the plan is currently in
> possession of the required ACL_EXECUTE permission. In the unlikely
> event that a superuser did revoke public execute permission on one of
> the range-minus functions, they might expect that said function would
> also no longer be callable by this mechanism. Even if they did not
> expect that, it's not an unreasonable consequence. Conversely, if it
> happens to become possible for a non-superuser to choose the called
> function here at some point in the future, then failure to check
> permissions at this point in the code would be a CVE.

If we assume the type support function approach, I think we don't
normally check permissions on type support functions. It is implied
that if you make use of a type, then the type support functions could be
executed at any time without further permission checking.

Maybe we should at least expand the comment to remind future developers
of this concern, though?