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
