1
0
mirror of https://github.com/postgres/postgres.git synced 2025-05-31 03:21:24 +03:00

Ensure that AFTER triggers run as the instigating user.

With deferred triggers, it is possible that the current role changes
between the time when the trigger is queued and the time it is
executed (for example, the triggering data modification could have
been executed in a SECURITY DEFINER function).

Up to now, deferred trigger functions would run with the current role
set to whatever was active at commit time.  That does not matter for
foreign-key constraints, whose correctness doesn't depend on the
current role.  But for user-written triggers, the current role
certainly can matter.

Hence, fix things so that AFTER triggers are fired under the role
that was active when they were queued, matching the behavior of
BEFORE triggers which would have actually fired at that time.
(If the trigger function is marked SECURITY DEFINER, that of course
overrides this, as it always has.)

This does not create any new security exposure: if you do DML on a
table owned by a hostile user, that user has always had various ways
to exploit your permissions, such as the aforementioned BEFORE
triggers, default expressions, etc.  It might remove some security
exposure, because the old behavior could potentially expose some
other role besides the one directly modifying the table.

There was discussion of making a larger change, such as running as
the trigger's owner.  However, that would break the common idiom of
capturing the value of CURRENT_USER in a trigger for auditing/logging
purposes.  This change will make no difference in the typical scenario
where the current role doesn't change before commit.

Arguably this is a bug fix, but it seems too big a semantic change
to consider for back-patching.

Author: Laurenz Albe <laurenz.albe@cybertec.at>
Reviewed-by: Joseph Koshakow <koshy44@gmail.com>
Reviewed-by: Pavel Stehule <pavel.stehule@gmail.com>
Discussion: https://postgr.es/m/77ee784cf248e842f74588418f55c2931e47bd78.camel@cybertec.at
This commit is contained in:
Tom Lane 2025-01-23 12:25:45 -05:00
parent 4e7f62bc38
commit 01463e1ccc
4 changed files with 154 additions and 0 deletions

View File

@ -129,6 +129,10 @@
In all cases, a trigger is executed as part of the same transaction as
the statement that triggered it, so if either the statement or the
trigger causes an error, the effects of both will be rolled back.
Also, the trigger will always run in the security context of the role
that executed the statement that caused the trigger to fire, unless
the trigger function is defined as <literal>SECURITY DEFINER</literal>,
in which case it will run as the function owner.
</para>
<para>

View File

@ -3635,6 +3635,7 @@ typedef struct AfterTriggerSharedData
TriggerEvent ats_event; /* event type indicator, see trigger.h */
Oid ats_tgoid; /* the trigger's ID */
Oid ats_relid; /* the relation it's on */
Oid ats_rolid; /* role to execute the trigger */
CommandId ats_firing_id; /* ID for firing cycle */
struct AfterTriggersTableData *ats_table; /* transition table access */
Bitmapset *ats_modifiedcols; /* modified columns */
@ -4117,6 +4118,7 @@ afterTriggerAddEvent(AfterTriggerEventList *events,
newshared->ats_firing_id == 0 &&
newshared->ats_table == evtshared->ats_table &&
newshared->ats_relid == evtshared->ats_relid &&
newshared->ats_rolid == evtshared->ats_rolid &&
bms_equal(newshared->ats_modifiedcols,
evtshared->ats_modifiedcols))
break;
@ -4289,6 +4291,8 @@ AfterTriggerExecute(EState *estate,
AfterTriggerShared evtshared = GetTriggerSharedData(event);
Oid tgoid = evtshared->ats_tgoid;
TriggerData LocTriggerData = {0};
Oid save_rolid;
int save_sec_context;
HeapTuple rettuple;
int tgindx;
bool should_free_trig = false;
@ -4492,6 +4496,17 @@ AfterTriggerExecute(EState *estate,
MemoryContextReset(per_tuple_context);
/*
* If necessary, become the role that was active when the trigger got
* queued. Note that the role might have been dropped since the trigger
* was queued, but if that is a problem, we will get an error later.
* Checking here would still leave a race condition.
*/
GetUserIdAndSecContext(&save_rolid, &save_sec_context);
if (save_rolid != evtshared->ats_rolid)
SetUserIdAndSecContext(evtshared->ats_rolid,
save_sec_context | SECURITY_LOCAL_USERID_CHANGE);
/*
* Call the trigger and throw away any possibly returned updated tuple.
* (Don't let ExecCallTriggerFunc measure EXPLAIN time.)
@ -4506,6 +4521,10 @@ AfterTriggerExecute(EState *estate,
rettuple != LocTriggerData.tg_newtuple)
heap_freetuple(rettuple);
/* Restore the current role if necessary */
if (save_rolid != evtshared->ats_rolid)
SetUserIdAndSecContext(save_rolid, save_sec_context);
/*
* Release resources
*/
@ -6431,6 +6450,7 @@ AfterTriggerSaveEvent(EState *estate, ResultRelInfo *relinfo,
(trigger->tginitdeferred ? AFTER_TRIGGER_INITDEFERRED : 0);
new_shared.ats_tgoid = trigger->tgoid;
new_shared.ats_relid = RelationGetRelid(rel);
new_shared.ats_rolid = GetUserId();
new_shared.ats_firing_id = 0;
if ((trigger->tgoldtable || trigger->tgnewtable) &&
transition_capture != NULL)

View File

@ -3748,3 +3748,70 @@ Inherits: parent
drop table parent, child;
drop function f();
-- Test who runs deferred trigger functions
-- setup
create role regress_groot;
create role regress_outis;
create function whoami() returns trigger language plpgsql
as $$
begin
raise notice 'I am %', current_user;
return null;
end;
$$;
alter function whoami() owner to regress_outis;
create table defer_trig (id integer);
grant insert on defer_trig to public;
create constraint trigger whoami after insert on defer_trig
deferrable initially deferred
for each row
execute function whoami();
-- deferred triggers must run as the user that queued the trigger
begin;
set role regress_groot;
insert into defer_trig values (1);
reset role;
set role regress_outis;
insert into defer_trig values (2);
reset role;
commit;
NOTICE: I am regress_groot
NOTICE: I am regress_outis
-- security definer functions override the user who queued the trigger
alter function whoami() security definer;
begin;
set role regress_groot;
insert into defer_trig values (3);
reset role;
commit;
NOTICE: I am regress_outis
alter function whoami() security invoker;
-- make sure the current user is restored after error
create or replace function whoami() returns trigger language plpgsql
as $$
begin
raise notice 'I am %', current_user;
perform 1 / 0;
return null;
end;
$$;
begin;
set role regress_groot;
insert into defer_trig values (4);
reset role;
commit; -- error expected
NOTICE: I am regress_groot
ERROR: division by zero
CONTEXT: SQL statement "SELECT 1 / 0"
PL/pgSQL function whoami() line 4 at PERFORM
select current_user = session_user;
?column?
----------
t
(1 row)
-- clean up
drop table defer_trig;
drop function whoami();
drop role regress_outis;
drop role regress_groot;

View File

@ -2839,3 +2839,66 @@ alter trigger parenttrig on parent rename to anothertrig;
drop table parent, child;
drop function f();
-- Test who runs deferred trigger functions
-- setup
create role regress_groot;
create role regress_outis;
create function whoami() returns trigger language plpgsql
as $$
begin
raise notice 'I am %', current_user;
return null;
end;
$$;
alter function whoami() owner to regress_outis;
create table defer_trig (id integer);
grant insert on defer_trig to public;
create constraint trigger whoami after insert on defer_trig
deferrable initially deferred
for each row
execute function whoami();
-- deferred triggers must run as the user that queued the trigger
begin;
set role regress_groot;
insert into defer_trig values (1);
reset role;
set role regress_outis;
insert into defer_trig values (2);
reset role;
commit;
-- security definer functions override the user who queued the trigger
alter function whoami() security definer;
begin;
set role regress_groot;
insert into defer_trig values (3);
reset role;
commit;
alter function whoami() security invoker;
-- make sure the current user is restored after error
create or replace function whoami() returns trigger language plpgsql
as $$
begin
raise notice 'I am %', current_user;
perform 1 / 0;
return null;
end;
$$;
begin;
set role regress_groot;
insert into defer_trig values (4);
reset role;
commit; -- error expected
select current_user = session_user;
-- clean up
drop table defer_trig;
drop function whoami();
drop role regress_outis;
drop role regress_groot;