Re: Fixing insecure security definer functions

On 3/29/07, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> As was pointed out awhile ago
> http://archives.postgresql.org/pgsql-general/2007-02/msg00673.php
> it's insecure to run a SECURITY DEFINER function with a search_path
> setting that's under the control of someone who wishes to subvert
> the function.  Even for non-security-definer functions, it seems
> useful to be able to select the search path for the function to use;
> we've had requests for that before.  Right now, this is possible but
> tedious and slow, because you really have to use a subtransaction
> to ensure that the path is reset correctly on exit:
>
>         BEGIN
>           SET LOCAL search_path = ...;
>           ... useful work here ...
>         EXCEPTION
>         END
>
> (In fact it's worse than that, since you can't write an EXCEPTION
> without at least one WHEN clause, which is maybe something to change?)
> Also, this approach isn't available in plain SQL functions.
>
> I would like to fix this for 8.3.  I don't have a patch yet but want
> to get buy-in on a design before feature freeze.  I propose the
> following, fully-backward-compatible design:
>
> 1. Add a text column "propath" to pg_proc.  It can be either NULL or
> a search path specification (interpreted the same as values for the
> search_path GUC variable).  NULL means use the caller's setting, ie,
> current behavior.
>
> 2. When fmgr.c sees either prosecdef or propath set for a function to be
> called, it will insert the fmgr_security_definer hook into the call.
> fmgr_security_definer will be responsible for establishing the correct
> current-user and/or path settings and restoring them on exit.  (We could
> use two independent hooks, but since these features will often be used
> together, implementing both with just one hook seems reasonable.)
>
> 3. Add optional clauses to CREATE FUNCTION and ALTER FUNCTION to specify
> the propath value.  I suggest, but am not wedded to,
>         PATH 'foo, bar'
>         PATH NONE
> Since PATH NONE is the default, it's not really needed in CREATE
> FUNCTION, but it seems useful to allow it for ALTER FUNCTION.

fwiw, I think this is a great solution...because the default behavior
is preserved you get through without any extra guc settings (although
you may want to add one anyways).

maybe security definer functions should raise a warning for implicit
PATH NONE, and possibly even deprecate that behavior and force people
to type it out in future (8.4+) releases.

merlin