Обсуждение: Security leak with trigger functions?

Permissions on a trigger function seem not to be checked,
and I can execute a function for which I have no privileges.

I consider this a security leak - or am I missing something?

Here is a complete example:

As superuser, create a trigger function that selects from pg_authid
with SECURITY INVOKER, and REVOKE EXECUTE from public:

test=# \c test postgres
You are now connected to database "test" as user "postgres".
test=# CREATE OR REPLACE FUNCTION insert_oid() RETURNS trigger AS
test-# $$BEGIN SELECT oid INTO NEW.useroid FROM pg_catalog.pg_authid
WHERE rolname = user; RETURN NEW; END;$$
test-# LANGUAGE plpgsql STABLE STRICT SECURITY DEFINER;
CREATE FUNCTION
test=# REVOKE EXECUTE ON FUNCTION insert_oid() FROM public;
REVOKE
test=# SELECT proacl FROM pg_catalog.pg_proc WHERE proname =
'insert_oid';       proacl
-----------------------{postgres=X/postgres}
(1 row)

As normal user, try to execute the function or select from
pg_catalog.pg_authid directly; both fail as expected:

test=# \c test laurenz
You are now connected to database "test" as user "laurenz".
test=> SELECT insert_oid();
ERROR:  permission denied for function insert_oid
test=> SELECT oid FROM pg_catalog.pg_authid WHERE rolname = user;
ERROR:  permission denied for relation pg_authid

Create a temporary table, define a trigger BEFORE INSERT using the
function that we cannot execute:

test=> CREATE TEMP TABLE lautest (id integer PRIMARY KEY, useroid oid
NOT NULL);
NOTICE:  CREATE TABLE / PRIMARY KEY will create implicit index
"lautest_pkey" for table "lautest"
CREATE TABLE
test=> CREATE TRIGGER insert_oid BEFORE INSERT ON lautest FOR EACH ROW
EXECUTE PROCEDURE insert_oid();
CREATE TRIGGER

Insert a row into the table.
The trigger function is executed, and I have selected a value from
pg_authid!

test=> INSERT INTO lautest (id) VALUES (1);
INSERT 0 1
test=> SELECT * FROM lautest;id | useroid
----+--------- 1 |      10
(1 row)

Yours,
Laurenz Albe

"Albe Laurenz" <all@adv.magwien.gv.at> writes:
> Permissions on a trigger function seem not to be checked,
> and I can execute a function for which I have no privileges.

Only if it's a trigger function, but I agree this is not very good.
The question in my mind is what privilege to check and when.

I believe the check probably ought to be whether the table owner can
call the function (certainly not the person doing the command that
invokes the trigger).  However, that raises the question of whether
having a separate TRIGGER privilege to attach triggers to tables isn't
itself a security hole: it means someone who might not himself have call
privileges could cause other people to call a function.  We just removed
the separate RULE privilege, and said you must be table owner to put a
rule on a table, for exactly analogous reasons.

The other question is when to check it.  If we check only at CREATE
TRIGGER time then there's the problem that revoking execute privilege
on a trigger function would not do what you'd expect.  OTOH checking
at every trigger call seems quite unpleasant.  Would it be all right
to check all the triggers once at statement start (eg, in
ExecBSInsertTriggers) whether or not they actually get called?

BTW, I just noticed another bug in the trigger stuff: in a statement
with multiple target relations, such as UPDATE across an inheritance
tree, ExecBSUpdateTriggers and friends get called only on the first
(parent) target relation.  Hence any statement-level triggers on child
tables aren't invoked.  This probably isn't very critical today, but
it would be if we use those functions to enforce permissions checks.
        regards, tom lane

Tom Lane wrote:
> The question in my mind is what privilege to check and when.

By extrapolation of the SQL standard, I'd say we'd need to check the 
EXECUTE privilege of the function at run time.

-- 
Peter Eisentraut
http://developer.postgresql.org/~petere/

Peter Eisentraut <peter_e@gmx.net> writes:
> Tom Lane wrote:
>> The question in my mind is what privilege to check and when.

> By extrapolation of the SQL standard, I'd say we'd need to check the 
> EXECUTE privilege of the function at run time.

Certainly EXECUTE privilege is what to check, but whose privilege?
        regards, tom lane

Tom Lane wrote:
> Peter Eisentraut <peter_e@gmx.net> writes:
> > Tom Lane wrote:
> >> The question in my mind is what privilege to check and when.
> >
> > By extrapolation of the SQL standard, I'd say we'd need to check
> > the EXECUTE privilege of the function at run time.
>
> Certainly EXECUTE privilege is what to check, but whose privilege?

SQL allows a trigger action to be a more or less random list of 
statements, which are checked at trigger run time against the 
privileges of the owner of the trigger.

("The authorization identifier of the owner of the schema that includes 
the trigger descriptor of TR is pushed onto the authorization stack.")

PostgreSQL only allows a trigger action of "call this function", so in 
the SQL standard context that would mean we'd need to check the EXECUTE 
privilege of the owner of the trigger.  The trick is figuring out who 
the owner is.  If it's the owner of the table, then TRIGGER privilege 
is effectively total control over the owner of the table.  If it's 
whoever created the trigger, it might be useful, but I don't see how 
that is compatible with the intent of the SQL standard.

-- 
Peter Eisentraut
http://developer.postgresql.org/~petere/

Peter,

> PostgreSQL only allows a trigger action of "call this function", so in
> the SQL standard context that would mean we'd need to check the EXECUTE
> privilege of the owner of the trigger.  The trick is figuring out who
> the owner is.  If it's the owner of the table, then TRIGGER privilege
> is effectively total control over the owner of the table.  If it's
> whoever created the trigger, it might be useful, but I don't see how
> that is compatible with the intent of the SQL standard.

If that's the case, then a separate TRIGGER priveledge would seem to be 
superfluous.

One thing to think about, though; our model allows granting ALTER 
privelidge on a table to roles other than the table owner.  It would seem 
kind of inconsistent to be able to grant non-owner roles the ability to 
drop a column, but restrict only the owner to adding a trigger.  For one 
thing, if you have a non-owner role which has ALTER permission and wants 
to add an FK, how would that work?

-- 
--Josh

Josh Berkus
PostgreSQL @ Sun
San Francisco

Josh Berkus <josh@agliodbs.com> writes:
>> ... we'd need to check the EXECUTE
>> privilege of the owner of the trigger.  The trick is figuring out who
>> the owner is.  If it's the owner of the table, then TRIGGER privilege
>> is effectively total control over the owner of the table.

> If that's the case, then a separate TRIGGER priveledge would seem to be 
> superfluous.

Yeah, you could make a good case for removing TRIGGER privilege and
making it be an ownership check, as we just did for RULE privilege.

> One thing to think about, though; our model allows granting ALTER 
> privelidge on a table to roles other than the table owner.

Huh?  ALTER requires ownership.
        regards, tom lane

Peter Eisentraut wrote:
> Tom Lane wrote:
>> Peter Eisentraut <peter_e@gmx.net> writes:
>>> Tom Lane wrote:
>>>> The question in my mind is what privilege to check and when.
>>>
>>> By extrapolation of the SQL standard, I'd say we'd need to check
>>> the EXECUTE privilege of the function at run time.
>>
>> Certainly EXECUTE privilege is what to check, but whose privilege?
>
> PostgreSQL only allows a trigger action of "call this function", so in

> the SQL standard context that would mean we'd need to check the
EXECUTE
> privilege of the owner of the trigger.  The trick is figuring out who
> the owner is.  If it's the owner of the table, then TRIGGER privilege
> is effectively total control over the owner of the table.  If it's
> whoever created the trigger, it might be useful, but I don't see how
> that is compatible with the intent of the SQL standard.

Looking at pg_trigger I have the impression that there is no such thing
as an 'owner of a trigger', and consequently the owner of the trigger
would automatically be the table owner.

I understand the reservations about the TRIGGER privilege, but I think
that it is obvious anyway that anybody who can add a trigger can
basically do everything with the table.

When adding a trigger, I would check if both the table owner and
the user who adds the trigger have EXECUTE privilege on the function.
That doesn't seem too restrictive to me.

For trigger execution, I see two options:
1) Check for EXECUTE privilege of the table owner at statement begin  time, as Tom Lane suggested. We cannot be sure if
thetrigger would  actually be executed, right? Should there be an error message even  when the trigger is not fired? Or
shouldthe trigger be silently  disabled? 
2) Whenever EXECUTE on a function is revoked, disable triggers on all  tables whose owners have now no longer execute
privilege. This should probably not be silent and require something like a  CASCADE option for REVOKE...  Also, there'd
haveto be an update whenever table ownership is  changed...  Seems quite difficult, but would save checking at runtime. 

Yours,
Laurenz Albe

Albe Laurenz wrote:
> Looking at pg_trigger I have the impression that there is no such thing
> as an 'owner of a trigger', and consequently the owner of the trigger
> would automatically be the table owner.
>
> I understand the reservations about the TRIGGER privilege, but I think
> that it is obvious anyway that anybody who can add a trigger can
> basically do everything with the table.
>
>   

Isn't the problem that they can do more than just things with the table? 
If the trigger runs as the owner of the table it can do *anything* the 
owner can do. So if we allow the alter privilege to include ability to 
place a trigger then that privilege includes everything the owner can do 
(including granting/revoking other privileges). Surely that is not what 
was intended. Arguably we should invent a concept of an explicit trigger 
owner.

cheers

andrew

On Fri, Dec 15, 2006 at 11:52:33AM -0500, Andrew Dunstan wrote:
> Isn't the problem that they can do more than just things with the table?
> If the trigger runs as the owner of the table it can do *anything* the
> owner can do. So if we allow the alter privilege to include ability to
> place a trigger then that privilege includes everything the owner can do
> (including granting/revoking other privileges). Surely that is not what
> was intended. Arguably we should invent a concept of an explicit trigger
> owner.

I thought the problem was the other way round. That some person created
a function as SECURITY DEFINER but restricted EXECUTE permissions. And
now anybody can create a table and use that function as a trigger and
it will be executed even though neither the owner of the table nor the
person executing the trigger has EXECUTE permissions.

Triggers don't have owners because like you said, the table owner
controls them. The point is that there's no check that the table owner
is actually allowed to execute the function being used as trigger.

The trigger never runs as the owner of the table AIUI, only ever as the
definer of the function or as session user.

Have a nice day,
--
Martijn van Oosterhout   <kleptog@svana.org>   http://svana.org/kleptog/
> From each according to his ability. To each according to his ability to litigate.

Martijn van Oosterhout wrote:
> On Fri, Dec 15, 2006 at 11:52:33AM -0500, Andrew Dunstan wrote:
>   
>> Isn't the problem that they can do more than just things with the table? 
>> If the trigger runs as the owner of the table it can do *anything* the 
>> owner can do. So if we allow the alter privilege to include ability to 
>> place a trigger then that privilege includes everything the owner can do 
>> (including granting/revoking other privileges). Surely that is not what 
>> was intended. Arguably we should invent a concept of an explicit trigger 
>> owner.
>>     
>
> I thought the problem was the other way round. That some person created
> a function as SECURITY DEFINER but restricted EXECUTE permissions. And
> now anybody can create a table and use that function as a trigger and
> it will be executed even though neither the owner of the table nor the
> person executing the trigger has EXECUTE permissions.
>
> Triggers don't have owners because like you said, the table owner
> controls them. The point is that there's no check that the table owner
> is actually allowed to execute the function being used as trigger.
>
> The trigger never runs as the owner of the table AIUI, only ever as the
> definer of the function or as session user.
>
>
>   

OK, sorry for the confusion.

cheers

andrew

Martijn van Oosterhout <kleptog@svana.org> writes:
> The trigger never runs as the owner of the table AIUI, only ever as the
> definer of the function or as session user.

Yeah.  This might itself be seen as a bug: I think you could make a
reasonable case that the default behavior ought to be to run as the
table owner (but still overridable if trigger function is SECURITY
DEFINER, of course).  In the current situation a table owner can use
a trigger function as a trojan horse against anyone modifying the
table.

And then there's the question of functions run as a result of rule
definitions.  That's a lot harder to fix, but (I think) triggers would
be relatively easy to do something about.
        regards, tom lane

Tom Lane wrote:
> Martijn van Oosterhout <kleptog@svana.org> writes:
>> The trigger never runs as the owner of the table AIUI, only ever as the
>> definer of the function or as session user.
> 
> Yeah.  This might itself be seen as a bug: I think you could make a
> reasonable case that the default behavior ought to be to run as the
> table owner (but still overridable if trigger function is SECURITY
> DEFINER, of course).  In the current situation a table owner can use
> a trigger function as a trojan horse against anyone modifying the
> table.

Is this true for on-select rules too? In that case, couldn't any
user run his code as postmaster by creating an appropriate on-select
rule and waiting until somebody/cron backups the database using pg_dump?

Or is pg_dump smart enough to skip dumping tables with on-select rules?

greetings, Florian Pflug

"Florian G. Pflug" <fgp@phlo.org> writes:
> Is this true for on-select rules too? In that case, couldn't any
> user run his code as postmaster by creating an appropriate on-select
> rule and waiting until somebody/cron backups the database using pg_dump?

I don't see any issue for views' on-select rules; they wouldn't get
executed during either dump or reload.

It does seem like there are some other potential hazards once you start
thinking this way:

* Datatype I/O functions: the output function will be run as superuser
during pg_dump, and the input function during restore.  I think this is
not an attack spot today because I/O functions can only be written in
C, but we'd have to think about the consequences before allowing I/O
functions in trusted P/L languages.  (Perhaps arrange for I/O functions
to be run as if setuid to their owner?  Could be expensive...)

* Functions associated with indexes would get run during restore:
both the datatype-related index support functions, and any functions
used in functional indexes.  This might be OK because we require
such functions to be immutable, but I do not think the link from
"immutable" to "can't write database" is currently air-tight.

* Functions in CHECK constraints (either table or domain constraints)
would be executed during restores.  There is not an immutability
constraint for these currently, although arguably it'd be reasonable
to require?

* Trigger functions: not executed during pg_dump, nor during a full
restore, but they *would* be executed during a data-only restore if
you'd not used --disable-triggers.

* ON INSERT rules: likewise, executed during data-only restores,
possibly resulting in execution of user-defined functions.

During restores, we normally set the userid to be the table owner while
loading data into a particular table, which would mostly close these
holes except that I think a function can revert the session
authorization to be whatever the outermost user id is.  Probably we need
to tighten up the conditions under which a SET SESSION AUTHORIZATION can
be reverted within a function.
        regards, tom lane

Added to TODO:

> * Tighten trigger permission checks
>
>   http://archives.postgresql.org/pgsql-hackers/2006-12/msg00564.php

and:

> * Tighten function permission checks
>
>  http://archives.postgresql.org/pgsql-hackers/2006-12/msg00568.php
>

---------------------------------------------------------------------------

Tom Lane wrote:
> "Florian G. Pflug" <fgp@phlo.org> writes:
> > Is this true for on-select rules too? In that case, couldn't any
> > user run his code as postmaster by creating an appropriate on-select
> > rule and waiting until somebody/cron backups the database using pg_dump?
> 
> I don't see any issue for views' on-select rules; they wouldn't get
> executed during either dump or reload.
> 
> It does seem like there are some other potential hazards once you start
> thinking this way:
> 
> * Datatype I/O functions: the output function will be run as superuser
> during pg_dump, and the input function during restore.  I think this is
> not an attack spot today because I/O functions can only be written in
> C, but we'd have to think about the consequences before allowing I/O
> functions in trusted P/L languages.  (Perhaps arrange for I/O functions
> to be run as if setuid to their owner?  Could be expensive...)
> 
> * Functions associated with indexes would get run during restore:
> both the datatype-related index support functions, and any functions
> used in functional indexes.  This might be OK because we require
> such functions to be immutable, but I do not think the link from
> "immutable" to "can't write database" is currently air-tight.
> 
> * Functions in CHECK constraints (either table or domain constraints)
> would be executed during restores.  There is not an immutability
> constraint for these currently, although arguably it'd be reasonable
> to require?
> 
> * Trigger functions: not executed during pg_dump, nor during a full
> restore, but they *would* be executed during a data-only restore if
> you'd not used --disable-triggers.
> 
> * ON INSERT rules: likewise, executed during data-only restores,
> possibly resulting in execution of user-defined functions.
> 
> During restores, we normally set the userid to be the table owner while
> loading data into a particular table, which would mostly close these
> holes except that I think a function can revert the session
> authorization to be whatever the outermost user id is.  Probably we need
> to tighten up the conditions under which a SET SESSION AUTHORIZATION can
> be reverted within a function.
> 
>             regards, tom lane
> 
> ---------------------------(end of broadcast)---------------------------
> TIP 5: don't forget to increase your free space map settings

--  Bruce Momjian   bruce@momjian.us EnterpriseDB    http://www.enterprisedb.com
 + If your life is a hard drive, Christ can be your backup. +

On 12/14/2006 01:17 PM, Peter Eisentraut wrote:
> Tom Lane wrote:
>> Peter Eisentraut <peter_e@gmx.net> writes:
>>> By extrapolation of the SQL standard, I'd say we'd need to check
>>> the EXECUTE privilege of the function at run time.
>>
>> Certainly EXECUTE privilege is what to check, but whose privilege?
> 
> ...
> ("The authorization identifier of the owner of the schema that includes 
> the trigger descriptor of TR is pushed onto the authorization stack.")
> 
> PostgreSQL only allows a trigger action of "call this function", so in 
> the SQL standard context that would mean we'd need to check the EXECUTE 
> privilege of the owner of the trigger.  The trick is figuring out who 
> the owner is.  If it's the owner of the table, then TRIGGER privilege 
> is effectively total control over the owner of the table.  If it's 
> whoever created the trigger, it might be useful, but I don't see how 
> that is compatible with the intent of the SQL standard.

Hmm, it's been not quite a dozen years, have there been later threads
that followed up on this discussion?

Is it still accurate to describe the status quo as:

- Triggers can be created only by a role with TRIGGER on the
  relation and EXECUTE on the function, checked at trigger creation
  time.

- Once created, triggers are executed with no local userid change (so,
  just as whatever role issued whatever DML fired the trigger), and
  without a check for EXECUTE on the function at that time, except:

- Internal trigger functions implementing RI checks will execute their
  queries as "the owner of whichever table the query touches (the
  referenced table for verification of FK inserts, the referencing
  table when cascading a PK change)."
  https://www.postgresql.org/message-id/5761.1509733215%40sss.pgh.pa.us

It seems to me that the 2006 conversation was complicated by the
fact that PostgreSQL triggers don't have owners of their own, and
don't have schema-qualified names, as triggers in the SQL standard do.
So the conversation was going in the direction of running as the owner
of the relation, but for the obvious consequence that it would turn
TRIGGER permission into an equivalent of the relation owner's identity.

If the idea of changing the current behavior at all could be thinkable,
could one think about changing it more along SQL standard lines, by
letting a trigger itself have a schema, and executing as the owner
of the schema?

This seems to me not too different from just letting a trigger have
an owner, and executing as that, which could seem less roundabout.
OTOH, going through the schema has the advantage of following the
standard, and may allow a little more flexibility in setting up
delegated authority; a schema with a certain owner can be created,
CREATE(*) on that schema can be selectively granted, enabling grantees
to create trigger declarations in that schema, applying to any
relations they have TRIGGER permission for. Either way, TRIGGER
permission is left with something useful to mean, unlike the
execute-as-relation-owner case, where the meaning of TRIGGER sort of
evaporates.

(*) a different permission might be better here, as many existing
schemas probably grant CREATE to roles today without intending that
they be able to create triggers that execute as the schema owner.
TRIGGER might work, having no existing meaning on a schema.

I think the execute-as-relation-owner idea might not easily fit
situations where different roles with cross-cutting concerns all have
their own reasons to put triggers on a given relation, but should not
otherwise have the same permissions or run with the same id. I think
such situations could easily be accommodated in a model where triggers
have schemas and execute as the schema owner.

I could imagine a graceful migration plan:

1. Give pg_trigger a tgnamespace column, but allow it to be null
   or InvalidOid, meaning a trigger that should have the current
   behavior, executed with no local userid change. Conversely, a
   trigger with a valid tgnamespace gets executed as the owner
   of that schema.

2. pg_upgrade preserves existing triggers as triggers with no
   namespace. The grammar is altered to allow a schema when creating
   a trigger (and to allow ALTER TRIGGER SET SCHEMA).

3. Permission checks for schemaless triggers stay as they are now:
   EXECUTE on the function checked at for the trigger's creator at
   creation time, but not at run time. For triggers in schemas, the
   schema owner is who must have EXECUTE on the function.

4. It could still be possible to avoid runtime permission checks,
   the existence of EXECUTE for the right role being checked at
   trigger creation time, if REVOKE EXECUTE on a trigger function
   includes a check through pg_depend for triggers whose execute
   right would be lost. Those would be (a) any triggers with schemas
   owned by a role having EXECUTE revoked, and (b) any schema-less
   triggers at all (as there's no telling whose permission might
   have been checked at the time they were created).

The simple check in 4(b) could be implemented on its own, without any
of the rest, as a simple way to plug the "security leak" in the status
quo.

Thoughts?

-Chap

On 1/22/18 16:04, Chapman Flack wrote:
>> PostgreSQL only allows a trigger action of "call this function", so in 
>> the SQL standard context that would mean we'd need to check the EXECUTE 
>> privilege of the owner of the trigger.  The trick is figuring out who 
>> the owner is.  If it's the owner of the table, then TRIGGER privilege 
>> is effectively total control over the owner of the table.  If it's 
>> whoever created the trigger, it might be useful, but I don't see how 
>> that is compatible with the intent of the SQL standard.
> 
> Hmm, it's been not quite a dozen years, have there been later threads
> that followed up on this discussion?

No, I don't think anything has changed here.

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services