Обсуждение: Wrong security context for deferred triggers?

Create a table and a deferrable constraint trigger:

 CREATE TABLE tab (i integer);

 CREATE FUNCTION trig() RETURNS trigger
    LANGUAGE plpgsql AS
 $$BEGIN
    RAISE NOTICE 'current_user = %', current_user;
    RETURN NEW;
 END;$$;

 CREATE CONSTRAINT TRIGGER trig AFTER INSERT ON tab
    DEFERRABLE INITIALLY IMMEDIATE
    FOR EACH ROW EXECUTE FUNCTION trig();

Create a role and allow it INSERT on the table:

 CREATE ROLE duff;

 GRANT INSERT ON tab TO duff;

Now become that role and try some inserts:

 SET ROLE duff;

 BEGIN;

 INSERT INTO tab VALUES (1);
 NOTICE:  current_user = duff

That looks ok; the current user is "duff".

 SET CONSTRAINTS ALL DEFERRED;

 INSERT INTO tab VALUES (2);

Become a superuser again and commit:

 RESET ROLE;

 COMMIT;
 NOTICE:  current_user = postgres


So a deferred constraint trigger does not run with the same security context
as an immediate trigger.  This is somewhat nasty in combination with
SECURITY DEFINER functions: if that function performs an operation, and that
operation triggers a deferred trigger, that trigger will run in the wrong
security context.

This behavior looks buggy to me.  What do you think?
I cannot imagine that it is a security problem, though.

Yours,
Laurenz Albe

On Mon, 2023-11-06 at 14:23 +0100, Laurenz Albe wrote:
>  CREATE FUNCTION trig() RETURNS trigger
>     LANGUAGE plpgsql AS
>  $$BEGIN
>     RAISE NOTICE 'current_user = %', current_user;
>     RETURN NEW;
>  END;$$;
>
>  CREATE CONSTRAINT TRIGGER trig AFTER INSERT ON tab
>     DEFERRABLE INITIALLY IMMEDIATE
>     FOR EACH ROW EXECUTE FUNCTION trig();
>
>  SET ROLE duff;
>
>  BEGIN;
>
>  INSERT INTO tab VALUES (1);
>  NOTICE:  current_user = duff
>
> That looks ok; the current user is "duff".
>
>  SET CONSTRAINTS ALL DEFERRED;
>
>  INSERT INTO tab VALUES (2);
>
> Become a superuser again and commit:
>
>  RESET ROLE;
>
>  COMMIT;
>  NOTICE:  current_user = postgres
>
>
> So a deferred constraint trigger does not run with the same security context
> as an immediate trigger.  This is somewhat nasty in combination with
> SECURITY DEFINER functions: if that function performs an operation, and that
> operation triggers a deferred trigger, that trigger will run in the wrong
> security context.
>
> This behavior looks buggy to me.  What do you think?
> I cannot imagine that it is a security problem, though.

I just realized one problem with running a deferred constraint trigger as
the triggering role: that role might have been dropped by the time the trigger
executes.  But then we could still error out.

Yours,
Laurenz Albe

On 11/6/23 14:23, Laurenz Albe wrote:
> ...
> 
> This behavior looks buggy to me.  What do you think?
> I cannot imagine that it is a security problem, though.
> 

How could code getting executed under the wrong role not be a security
issue? Also, does this affect just the role, or are there some other
settings that may unexpectedly change (e.g. search_path)?


regards

-- 
Tomas Vondra
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On Mon, 2023-11-06 at 18:29 +0100, Tomas Vondra wrote:
> On 11/6/23 14:23, Laurenz Albe wrote:
> > This behavior looks buggy to me.  What do you think?
> > I cannot imagine that it is a security problem, though.
>
> How could code getting executed under the wrong role not be a security
> issue? Also, does this affect just the role, or are there some other
> settings that may unexpectedly change (e.g. search_path)?

Perhaps it is a security issue, and I am just lacking imagination.

Yes, changes to "search_path" should also have an effect.

Yours,
Laurenz Albe

On Mon, 2023-11-06 at 18:29 +0100, Tomas Vondra wrote:
> On 11/6/23 14:23, Laurenz Albe wrote:
> > This behavior looks buggy to me.  What do you think?
> > I cannot imagine that it is a security problem, though.
>
> How could code getting executed under the wrong role not be a security
> issue? Also, does this affect just the role, or are there some other
> settings that may unexpectedly change (e.g. search_path)?

Here is a patch that fixes this problem by keeping track of the
current role in the AfterTriggerSharedData.

I have thought some more about the security aspects:

1. With the new code, you could call a SECURITY DEFINER function
   that modifies data on a table with a deferred trigger, then
   modify the trigger function before you commit and have your
   code run with elevated privileges.
   But I think that we need not worry about that.  If a
   superuser performs DML on a table that an untrusted user
   controls, all bets are off anyway.  The attacker might as
   well put the bad code into the trigger *before* calling the
   SECURITY DEFINER function.

2. The more serious concern is that the old code constitutes
   a narrow security hazard: a superuser could temporarily
   assume an unprivileged role to avoid risks while performing
   DML on a table controlled by an untrusted user, but for
   some reason resume being a superuser *before* COMMIT.
   Then a deferred trigger would inadvertently run with
   superuser privileges.

   That seems like a very unlikely scenario (who would RESET ROLE
   before committing in that situation?).  Moreover, it seems
   like the current buggy behavior has been in place for decades
   without anybody noticing.

   I am not against backpatching this (the fix is simple enough),
   but I am not convinced that it is necessary.

Yours,
Laurenz Albe