Re: Fixing insecure security definer functions

* Peter Eisentraut (peter_e@gmx.net) wrote:
> Regarding the advisory on possibly insecure security definer functions
> that I just sent out (by overriding the search path you can make the
> function do whatever you want with the privileges of the function
> owner), the favored solution after some initial discussion in the core
> team was to save the search path at creation time with each function.

Would this be done only on security-definer functions?

> This measure will arguably also increase the robustness of functions in
> general, and it seems to be desirable as part of the effort to make
> plan invalidation work.
>
> Quite probably, there will be all sorts of consequences in terms of
> backward compatibility and preserving the possibility of valid uses of
> the old behavior and so on.  So I'm inviting input on how to fix the
> problem in general and how to avoid the mentioned follow-up problems in
> particular.

It'll break most of the functions that we have in our production
systems...  They're not security definer functions but it's routine for
us to switch between different schemas to run a function on.  I
certainly don't want to have to have seperate functions for these either
as it'd be completely duplicated code with just different search paths
set.  I'm honestly not the least bit impressed with this solution to
the problem.

What about pushing all the in-function references down to the
specific objects referenced at plan creation time (err, I thought this
was done?)?  In most cases what we're doing is building up queries and
then running them with execute so I *think* that'd still work for us.
Also, it seems like that's the way to deal with the plan-invocation
problem (since that's really going to have to go down to object level
anyway eventually, isn't it?) rather than trying to handle using the
search path to figure out if it's invalidated now or not based on what's
currently there.

Note that I'm not suggesting users would change their source code for
this but rather that the 'create function' command would implicitly do
this ala what create view does.  I really could have sworn something
like this was done (where OIDs are saved).

Another option might be to modify the 'create function' syntax to have
an option of 'search path' to set what search path the function should
have at the start.  Then, for security definer functions at least, issue
a WARNING if that isn't being set at CREATE FUNCTION time.  I'm pretty
sure that in most cases (certainly for us) that'd be noticed and at
least investigated.  Another option would be to ERROR if it's not
provided and allow the previous behaviour by allowing it to be set to
NULL (again, mainly on security definer functions..  maybe warning on
others or something).
Hope these thoughts help...
    Stephen