Re: 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(at)adv(dot)magwien(dot)gv(dot)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(at)gmx(dot)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(at)gmx(dot)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(at)agliodbs(dot)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

On 12/14/2006 01:17 PM, Peter Eisentraut wrote:
> Tom Lane wrote:
>> Peter Eisentraut <peter_e(at)gmx(dot)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