Re: Wrong security context for deferred triggers?

Laurenz Albe <laurenz.albe@cybertec.at>

From: Laurenz Albe <laurenz.albe@cybertec.at>
To: Tomas Vondra <tomas.vondra@enterprisedb.com>, pgsql-hackers@lists.postgresql.org
Date: 2024-03-06T13:32:06Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →
  1. Doc: improve description of which role runs a trigger.

  2. Change role names used in trigger test.

  3. Ensure that AFTER triggers run as the instigating user.

  4. Reverse the search order in afterTriggerAddEvent().

Attachments

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