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.
Thoughts?
--
Robert Haas
EDB: http://www.enterprisedb.com
