Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit01463e1

Browse files
committed
Ensure that AFTER triggers run as the instigating user.
With deferred triggers, it is possible that the current role changesbetween the time when the trigger is queued and the time it isexecuted (for example, the triggering data modification could havebeen executed in a SECURITY DEFINER function).Up to now, deferred trigger functions would run with the current roleset to whatever was active at commit time. That does not matter forforeign-key constraints, whose correctness doesn't depend on thecurrent role. But for user-written triggers, the current rolecertainly can matter.Hence, fix things so that AFTER triggers are fired under the rolethat was active when they were queued, matching the behavior ofBEFORE triggers which would have actually fired at that time.(If the trigger function is marked SECURITY DEFINER, that of courseoverrides this, as it always has.)This does not create any new security exposure: if you do DML on atable owned by a hostile user, that user has always had various waysto exploit your permissions, such as the aforementioned BEFOREtriggers, default expressions, etc. It might remove some securityexposure, because the old behavior could potentially expose someother role besides the one directly modifying the table.There was discussion of making a larger change, such as running asthe trigger's owner. However, that would break the common idiom ofcapturing the value of CURRENT_USER in a trigger for auditing/loggingpurposes. This change will make no difference in the typical scenariowhere the current role doesn't change before commit.Arguably this is a bug fix, but it seems too big a semantic changeto 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
1 parent4e7f62b commit01463e1

File tree

4 files changed

+154
-0
lines changed

4 files changed

+154
-0
lines changed

‎doc/src/sgml/trigger.sgml‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,10 @@
129129
In all cases, a trigger is executed as part of the same transaction as
130130
the statement that triggered it, so if either the statement or the
131131
trigger causes an error, the effects of both will be rolled back.
132+
Also, the trigger will always run in the security context of the role
133+
that executed the statement that caused the trigger to fire, unless
134+
the trigger function is defined as <literal>SECURITY DEFINER</literal>,
135+
in which case it will run as the function owner.
132136
</para>
133137

134138
<para>

‎src/backend/commands/trigger.c‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3635,6 +3635,7 @@ typedef struct AfterTriggerSharedData
36353635
TriggerEventats_event;/* event type indicator, see trigger.h */
36363636
Oidats_tgoid;/* the trigger's ID */
36373637
Oidats_relid;/* the relation it's on */
3638+
Oidats_rolid;/* role to execute the trigger */
36383639
CommandIdats_firing_id;/* ID for firing cycle */
36393640
structAfterTriggersTableData*ats_table;/* transition table access */
36403641
Bitmapset*ats_modifiedcols;/* modified columns */
@@ -4117,6 +4118,7 @@ afterTriggerAddEvent(AfterTriggerEventList *events,
41174118
newshared->ats_firing_id==0&&
41184119
newshared->ats_table==evtshared->ats_table&&
41194120
newshared->ats_relid==evtshared->ats_relid&&
4121+
newshared->ats_rolid==evtshared->ats_rolid&&
41204122
bms_equal(newshared->ats_modifiedcols,
41214123
evtshared->ats_modifiedcols))
41224124
break;
@@ -4289,6 +4291,8 @@ AfterTriggerExecute(EState *estate,
42894291
AfterTriggerSharedevtshared=GetTriggerSharedData(event);
42904292
Oidtgoid=evtshared->ats_tgoid;
42914293
TriggerDataLocTriggerData= {0};
4294+
Oidsave_rolid;
4295+
intsave_sec_context;
42924296
HeapTuplerettuple;
42934297
inttgindx;
42944298
boolshould_free_trig= false;
@@ -4492,6 +4496,17 @@ AfterTriggerExecute(EState *estate,
44924496

44934497
MemoryContextReset(per_tuple_context);
44944498

4499+
/*
4500+
* If necessary, become the role that was active when the trigger got
4501+
* queued. Note that the role might have been dropped since the trigger
4502+
* was queued, but if that is a problem, we will get an error later.
4503+
* Checking here would still leave a race condition.
4504+
*/
4505+
GetUserIdAndSecContext(&save_rolid,&save_sec_context);
4506+
if (save_rolid!=evtshared->ats_rolid)
4507+
SetUserIdAndSecContext(evtshared->ats_rolid,
4508+
save_sec_context |SECURITY_LOCAL_USERID_CHANGE);
4509+
44954510
/*
44964511
* Call the trigger and throw away any possibly returned updated tuple.
44974512
* (Don't let ExecCallTriggerFunc measure EXPLAIN time.)
@@ -4506,6 +4521,10 @@ AfterTriggerExecute(EState *estate,
45064521
rettuple!=LocTriggerData.tg_newtuple)
45074522
heap_freetuple(rettuple);
45084523

4524+
/* Restore the current role if necessary */
4525+
if (save_rolid!=evtshared->ats_rolid)
4526+
SetUserIdAndSecContext(save_rolid,save_sec_context);
4527+
45094528
/*
45104529
* Release resources
45114530
*/
@@ -6431,6 +6450,7 @@ AfterTriggerSaveEvent(EState *estate, ResultRelInfo *relinfo,
64316450
(trigger->tginitdeferred ?AFTER_TRIGGER_INITDEFERRED :0);
64326451
new_shared.ats_tgoid=trigger->tgoid;
64336452
new_shared.ats_relid=RelationGetRelid(rel);
6453+
new_shared.ats_rolid=GetUserId();
64346454
new_shared.ats_firing_id=0;
64356455
if ((trigger->tgoldtable||trigger->tgnewtable)&&
64366456
transition_capture!=NULL)

‎src/test/regress/expected/triggers.out‎

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3748,3 +3748,70 @@ Inherits: parent
37483748

37493749
drop table parent, child;
37503750
drop function f();
3751+
-- Test who runs deferred trigger functions
3752+
-- setup
3753+
create role regress_groot;
3754+
create role regress_outis;
3755+
create function whoami() returns trigger language plpgsql
3756+
as $$
3757+
begin
3758+
raise notice 'I am %', current_user;
3759+
return null;
3760+
end;
3761+
$$;
3762+
alter function whoami() owner to regress_outis;
3763+
create table defer_trig (id integer);
3764+
grant insert on defer_trig to public;
3765+
create constraint trigger whoami after insert on defer_trig
3766+
deferrable initially deferred
3767+
for each row
3768+
execute function whoami();
3769+
-- deferred triggers must run as the user that queued the trigger
3770+
begin;
3771+
set role regress_groot;
3772+
insert into defer_trig values (1);
3773+
reset role;
3774+
set role regress_outis;
3775+
insert into defer_trig values (2);
3776+
reset role;
3777+
commit;
3778+
NOTICE: I am regress_groot
3779+
NOTICE: I am regress_outis
3780+
-- security definer functions override the user who queued the trigger
3781+
alter function whoami() security definer;
3782+
begin;
3783+
set role regress_groot;
3784+
insert into defer_trig values (3);
3785+
reset role;
3786+
commit;
3787+
NOTICE: I am regress_outis
3788+
alter function whoami() security invoker;
3789+
-- make sure the current user is restored after error
3790+
create or replace function whoami() returns trigger language plpgsql
3791+
as $$
3792+
begin
3793+
raise notice 'I am %', current_user;
3794+
perform 1 / 0;
3795+
return null;
3796+
end;
3797+
$$;
3798+
begin;
3799+
set role regress_groot;
3800+
insert into defer_trig values (4);
3801+
reset role;
3802+
commit; -- error expected
3803+
NOTICE: I am regress_groot
3804+
ERROR: division by zero
3805+
CONTEXT: SQL statement "SELECT 1 / 0"
3806+
PL/pgSQL function whoami() line 4 at PERFORM
3807+
select current_user = session_user;
3808+
?column?
3809+
----------
3810+
t
3811+
(1 row)
3812+
3813+
-- clean up
3814+
drop table defer_trig;
3815+
drop function whoami();
3816+
drop role regress_outis;
3817+
drop role regress_groot;

‎src/test/regress/sql/triggers.sql‎

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2839,3 +2839,66 @@ alter trigger parenttrig on parent rename to anothertrig;
28392839

28402840
droptable parent, child;
28412841
dropfunction f();
2842+
2843+
-- Test who runs deferred trigger functions
2844+
2845+
-- setup
2846+
create role regress_groot;
2847+
create role regress_outis;
2848+
createfunctionwhoami() returns trigger language plpgsql
2849+
as $$
2850+
begin
2851+
raise notice'I am %',current_user;
2852+
returnnull;
2853+
end;
2854+
$$;
2855+
alterfunction whoami() owner to regress_outis;
2856+
2857+
createtabledefer_trig (idinteger);
2858+
grant inserton defer_trig to public;
2859+
createconstraint trigger whoami after inserton defer_trig
2860+
deferrable initially deferred
2861+
for each row
2862+
execute function whoami();
2863+
2864+
-- deferred triggers must run as the user that queued the trigger
2865+
begin;
2866+
set role regress_groot;
2867+
insert into defer_trigvalues (1);
2868+
reset role;
2869+
set role regress_outis;
2870+
insert into defer_trigvalues (2);
2871+
reset role;
2872+
commit;
2873+
2874+
-- security definer functions override the user who queued the trigger
2875+
alterfunction whoami() security definer;
2876+
begin;
2877+
set role regress_groot;
2878+
insert into defer_trigvalues (3);
2879+
reset role;
2880+
commit;
2881+
alterfunction whoami() security invoker;
2882+
2883+
-- make sure the current user is restored after error
2884+
create or replacefunctionwhoami() returns trigger language plpgsql
2885+
as $$
2886+
begin
2887+
raise notice'I am %',current_user;
2888+
perform1/0;
2889+
returnnull;
2890+
end;
2891+
$$;
2892+
2893+
begin;
2894+
set role regress_groot;
2895+
insert into defer_trigvalues (4);
2896+
reset role;
2897+
commit;-- error expected
2898+
selectcurrent_user=session_user;
2899+
2900+
-- clean up
2901+
droptable defer_trig;
2902+
dropfunction whoami();
2903+
drop role regress_outis;
2904+
drop role regress_groot;

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp