Thread: BEFORE triggers that return NULL can circumvent referentialintegrity

The purpose of a foreign key constraint is to enforce referential integrity
between the primary key that is referenced and the foreign key that
references it. In the PostgreSQL documentation foreign keys are described
using the following language:

"You want to make sure that no one can insert rows in the <fkey> table that
do not have a matching entry in the <pkey> table. This is called maintaining
the referential integrity of your data."

The non compliance issue reported previously (concerning the potential
firing of user-defined AFTER triggers prior to firing immediate-mode foreign
key constraint triggers) still guaranteed that the database would be left in
a consistent state at the end of each statement. Even though you could run
some SQL inside of a trigger that had access to inconsistent state, if that
state was inconsistent at the end of the statement the statement would still
fail; however, exploiting a PostgreSQL feature of BEFORE triggers allows
users to run SQL statements that do not fail or rollback, and that can
commit the database in a state that is inconsistent with the foreign key
constraints.

In the PostgreSQL documentation for BEFORE triggers there is a section
defining the behavior of a BEFORE trigger that returns NULL as follows:

"It can return NULL to skip the operation for the current row. This
instructs the executor to not perform the row-level operation that invoked
the trigger (the insertion, modification, or deletion of a particular table
row)."

This creates a problem in combination with referential actions because a
BEFORE trigger that returns NULL on the referencing side of the foreign key
constraint will block the referential action (such as a cascaded DELETE or
cascaded UPDATE) without causing the statement to fail, thereby breaking
referential integrity.

This issue is very serious because it allows the system to create and commit
state that violates the foreign key constraints. This is clearly not the
intended behavior according to the PostgreSQL documentation above, nor does
it comply with the ANSI-SQL standard.

Consider the following:

Example:

create table t1(id int primary key);
create table t2(id int primary key, t1_id int, foreign key(t1_id) references
t1(id) ON UPDATE CASCADE);

create or replace function trig_func() returns trigger
as $$
begin
    raise notice 'test';
    return null;
end;
$$ language plpgsql;

insert into t1 values(0);
insert into t2 values(0, 0);

create trigger trig BEFORE UPDATE on t2
    for each row execute procedure trig_func();

update t1 set id = 1 where id = 0;

select * from t1;
select * from t2;

What happens here is actually quite subtle:

1. The UPDATE to t1 occurs which will fire the foreign key constraint that
is implemented as an AFTER trigger.
* Since ON UPDATE CASCADE has been specified, the check for the constraint
will just attempt to do the UPDATE of col t1_id = 1 where t1_id = 0 on table
t2.
2. The UPDATE on t2 will begin and the **first** thing that will happen is
the BEFORE trigger **trig** will fire.
3. trig will do nothing but raise a notice 'test' and then return NULL.
4. Based on the behavior of BEFORE triggers as described in PostgreSQL
documentation above, the NULL return will **suppress** the cascaded UPDATE
of t2.t1_id.
* The UPDATE will complete and the foreign key constraint will have been
assumed to be maintained since a cascade was triggered, however since the
actual cascade action was suppressed by a BEFORE trigger we have reached an
inconsistent state.

The final two select statements will display:

select * from t1;
 id
----
  1
(1 row)


select * from t2;
 id | t1_id
----+-------
  0 |     0
(1 row)

Obviously the entry (0,0) in t2 does not correspond to any primary key entry
in t1 any longer, and we have therefore circumvented the integrity of
foreign keys in PostgreSQL by exploiting this behavior of BEFORE triggers.

The ANSI SQL Standard specifies the following in regards to trigger
execution:

When the <triggered SQL statement> TSS of TA is executed:
a) The <SQL procedure statement>s simply contained in TSS are effectively
executed in the
order in which they are specified in TSS.
b) If the execution of TSS is not successful, then an exception condition is
raised: trigger action
exception. The exception information associated with TSS is entered into the
diagnostics
area in a location other than the location corresponding to condition number
1 (one).

The semantics of RETURN NULL in a BEFORE trigger is a PostgreSQL extension
to the standard that can violate referential integrity. In order to continue
to support RETURN NULL in BEFORE triggers, we need to ensure that it cannot
be used to circumvent referential actions.

The code above was executed on PostgreSQL 10.5 on Linux.




-----
Jim Finnerty, AWS, Amazon Aurora PostgreSQL
--
Sent from: http://www.postgresql-archive.org/PostgreSQL-bugs-f2117394.html

Jim Finnerty <jfinnert@amazon.com> writes:
> The non compliance issue reported previously (concerning the potential
> firing of user-defined AFTER triggers prior to firing immediate-mode foreign
> key constraint triggers) still guaranteed that the database would be left in
> a consistent state at the end of each statement. Even though you could run
> some SQL inside of a trigger that had access to inconsistent state, if that
> state was inconsistent at the end of the statement the statement would still
> fail; however, exploiting a PostgreSQL feature of BEFORE triggers allows
> users to run SQL statements that do not fail or rollback, and that can
> commit the database in a state that is inconsistent with the foreign key
> constraints.

Yes.  This is known, and documented, and we don't really intend to change
it.

> This is clearly not the
> intended behavior according to the PostgreSQL documentation

What documentation?  If there's a contrary promise somewhere, we need
to fix that.  The CREATE TRIGGER page is pretty clear about it though:

    There is also nonstandard behavior if BEFORE triggers modify rows or
    prevent updates during an update that is caused by a referential
    action. This can lead to constraint violations or stored data that
    does not honor the referential constraint.

            regards, tom lane

re:     There is also nonstandard behavior if BEFORE triggers modify rows or
    prevent updates during an update that is caused by a referential
    action. This can lead to constraint violations or stored data that
    does not honor the referential constraint.

That supports the position that it was already documented, I'll grant you
that.  It is clearly a bug if it causes foreign key constraints to be
violated, documented or not.  Otherwise foreign key constraints mean nothing
in PostgreSQL.

I don't think this would be particularly difficult to fix so that it causes
the statement to fail when there is a referential action, rather than
silently returning and leaving the database in an inconsistent state.

What is the argument in favor of retaining behavior that renders foreign
keys meaningless?

What is the process for deciding whether a report represents an actionable
bug that ought to be fixed by the community?

    /Jim




-----
Jim Finnerty, AWS, Amazon Aurora PostgreSQL
--
Sent from: http://www.postgresql-archive.org/PostgreSQL-bugs-f2117394.html

Thank you, Pavel.  The initial issue is the willingness to modify the RETURN
NULL semantics so that it causes a statement to fail if the triggering event
was a referential action, thereby preserving both Foreign Key semantics and
RETURN NULL semantics in cases other than when it breaks referential
integrity.  In my opinion, extending SQL in a way that broke referential
integrity was a mistake, but it's a mistake that can be fixed without too
much impact on existing applications.

If the community is willing to accept that RETURN NULL needs this tweak to
its semantics, we can provide an implementation and will post it to
pghackers.



-----
Jim Finnerty, AWS, Amazon Aurora PostgreSQL
--
Sent from: http://www.postgresql-archive.org/PostgreSQL-bugs-f2117394.html