Re: Fixing insecure security definer functions

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: pgsql-hackers(at)postgresql(dot)org
Subject: Re: Fixing insecure security definer functions
Date: 2007-03-29 17:30:54
Message-ID: 24965.1175189454@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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.

Comments?

regards, tom lane

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Joshua D. Drake 2007-03-29 17:31:45 Re: Patch queue concern
Previous Message Bruce Momjian 2007-03-29 17:26:31 Re: Patch queue concern