Обсуждение: Referential integrity vulnerability in 8.3.3

Поиск
Список
Период
Сортировка

Referential integrity vulnerability in 8.3.3

От
"Sergey Konoplev"
Дата:
Hello community

There is an oddity (or a bug) in situation with returning null before
delete trigger and referential integrity in PG 8.3.3. I tryed to find
a solution in Google and PG documentation and have noticed nothing
useful.

Let's start from tables creation.

CREATE TABLE table1
(
  id serial NOT NULL,
  field1 text,
  CONSTRAINT table1_pk PRIMARY KEY (id)
);

CREATE TABLE table2
(
  id serial NOT NULL,
  table1_id integer,
  CONSTRAINT table2_pk PRIMARY KEY (id),
  CONSTRAINT table2_fk1 FOREIGN KEY (table1_id)
      REFERENCES table1 (id) MATCH SIMPLE
      ON UPDATE CASCADE ON DELETE CASCADE
);

Well.. Second one refrences first one and has to be updated and
deleted cascaded. Next create before delete trigger on tabe2 allways
returning null.

CREATE OR REPLACE FUNCTION tr_stop()
  RETURNS trigger AS
$BODY$begin
    return null;
end;$BODY$
  LANGUAGE 'plpgsql' VOLATILE;

CREATE TRIGGER tr_stop
  BEFORE DELETE
  ON table2
  FOR EACH ROW
  EXECUTE PROCEDURE tr_stop();

Inserting three rows into table1

insert into table1 (id, field1) values (1, 'qqq');
insert into table1 (id, field1) values (2, 'www');
insert into table1 (id, field1) values (3, 'eee');

and refer to them from table2.

insert into table2 (id, table1_id) values (1, 1);
insert into table2 (id, table1_id) values (2, 2);
insert into table2 (id, table1_id) values (3, 3);

Now comming to a head. As I supposed earlier, deletion from table1 has
to be prevented by referential integrity when the trigger prevents
deletion of refered row from table2. But it doesn't.

delete from table1;

It deletes all rows from table1 and doesn't touch rows from table2.

select * from table1
 id | field1
----+--------
(0 rows)

select * from table2 where not exists(select 1 from table1 where id =
table2.table1_id)
 id | table1_id
----+-----------
  1 |         1
  2 |         2
  3 |         3
(3 rows)

Will you explain me please why PG behave so cos IMHO it's a bit
illogical. Thanx.

p.s. Some info from pg_trigger below

select c.relname, t.* from pg_class c left join pg_trigger t on
t.tgrelid = c.oid
where relname in ('table1', 'table2') order by relname

 relname | tgrelid  |            tgname             |  tgfoid  |
tgtype | tgenabled | tgisconstraint | tgconstrname | tgconstrrelid |
tgconstraint | tgdeferrable | tginitdeferred | tgnargs | tgattr |
tgargs

---------+----------+-------------------------------+----------+--------+-----------+----------------+--------------+---------------+--------------+--------------+----------------+---------+--------+--------
 table1  | 55880268 | RI_ConstraintTrigger_55880305 |     1646 |
9 | O         | t              | table2_fk1   |      55880296 |
55880302 | f            | f              |       0 |        |
 table1  | 55880268 | RI_ConstraintTrigger_55880306 |     1647 |
17 | O         | t              | table2_fk1   |      55880296 |
55880302 | f            | f              |       0 |        |
 table2  | 55880296 | tr_stop                                 |
55881180 |     11 | O         | f              |              |
     0 |            0 | f            | f              |       0 |
  |
 table2  | 55880296 | RI_ConstraintTrigger_55880303 |     1644 |
5 | O         | t              | table2_fk1   |      55880268 |
55880302 | f            | f              |       0 |        |
 table2  | 55880296 | RI_ConstraintTrigger_55880304 |     1645 |
17 | O         | t              | table2_fk1   |      55880268 |
55880302 | f            | f              |       0 |        |
(5 rows)

--
Regards,
Sergey Konoplev

Re: Referential integrity vulnerability in 8.3.3

От
Richard Huxton
Дата:
Sergey Konoplev wrote:
> There is an oddity (or a bug) in situation with returning null before
> delete trigger and referential integrity in PG 8.3.3. I tryed to find
> a solution in Google and PG documentation and have noticed nothing
> useful.
[snip]
> CREATE OR REPLACE FUNCTION tr_stop()
>   RETURNS trigger AS
> $BODY$begin
>     return null;
> end;$BODY$
>   LANGUAGE 'plpgsql' VOLATILE;
>
> CREATE TRIGGER tr_stop
>   BEFORE DELETE
>   ON table2
>   FOR EACH ROW
>   EXECUTE PROCEDURE tr_stop();
[snip]
> Now comming to a head. As I supposed earlier, deletion from table1 has
> to be prevented by referential integrity when the trigger prevents
> deletion of refered row from table2. But it doesn't.
[snip]
> Will you explain me please why PG behave so cos IMHO it's a bit
> illogical. Thanx.

Your trigger doesn't prevent deletion, it just skips the row(s) in
question from being affected. Raise an exception if you want to abort
the transaction.

See the manual - triggers chapter and plpgsql chapter for more details.

--
   Richard Huxton
   Archonet Ltd

Re: Referential integrity vulnerability in 8.3.3

От
"Sergey Konoplev"
Дата:
Yes it is. But it the way to break integrity cos rows from table2
still refer to deleted rows from table1. So it conflicts with ideology isn't it?

On Tue, Jul 15, 2008 at 4:00 PM, Richard Huxton <dev@archonet.com> wrote:
> Sergey Konoplev wrote:
>>
>> There is an oddity (or a bug) in situation with returning null before
>> delete trigger and referential integrity in PG 8.3.3. I tryed to find
>> a solution in Google and PG documentation and have noticed nothing
>> useful.
>
> [snip]
>>
>> CREATE OR REPLACE FUNCTION tr_stop()
>>  RETURNS trigger AS
>> $BODY$begin
>>    return null;
>> end;$BODY$
>>  LANGUAGE 'plpgsql' VOLATILE;
>>
>> CREATE TRIGGER tr_stop
>>  BEFORE DELETE
>>  ON table2
>>  FOR EACH ROW
>>  EXECUTE PROCEDURE tr_stop();
>
> [snip]
>>
>> Now comming to a head. As I supposed earlier, deletion from table1 has
>> to be prevented by referential integrity when the trigger prevents
>> deletion of refered row from table2. But it doesn't.
>
> [snip]
>>
>> Will you explain me please why PG behave so cos IMHO it's a bit
>> illogical. Thanx.
>
> Your trigger doesn't prevent deletion, it just skips the row(s) in question
> from being affected. Raise an exception if you want to abort the
> transaction.
>
> See the manual - triggers chapter and plpgsql chapter for more details.
>
> --
>  Richard Huxton
>  Archonet Ltd
>



--
Regards,
Sergey Konoplev

Re: Referential integrity vulnerability in 8.3.3

От
Richard Huxton
Дата:
Sergey Konoplev wrote:
> Yes it is. But it the way to break integrity cos rows from table2
> still refer to deleted rows from table1. So it conflicts with
> ideology isn't it?

Yes, but I'm not sure you could have a sensible behaviour-modifying
BEFORE trigger without this loophole. Don't forget, ordinary users can't
work around this - you need suitable permissions.

You could rewrite PG's foreign-key code to check the referencing table
after the delete is supposed to have taken place, and make sure it has.
That's going to halve the speed of all your foreign-key checks though.

--
   Richard Huxton
   Archonet Ltd

Re: Referential integrity vulnerability in 8.3.3

От
"Sergey Konoplev"
Дата:
>>
>> Yes it is. But it the way to break integrity cos rows from table2 still
>> refer to deleted rows from table1. So it conflicts with
>> ideology isn't it?
>
> Yes, but I'm not sure you could have a sensible behaviour-modifying BEFORE
> trigger without this loophole. Don't forget, ordinary users can't work
> around this - you need suitable permissions.
>
> You could rewrite PG's foreign-key code to check the referencing table after
> the delete is supposed to have taken place, and make sure it has. That's
> going to halve the speed of all your foreign-key checks though.
>

I'm not sure I've understood you right, sorry. Does "rewrite PG's
foreign-key code" mean DDL? If it does how could I do this?

--
Regards,
Sergey Konoplev

Re: Referential integrity vulnerability in 8.3.3

От
Richard Huxton
Дата:
Sergey Konoplev wrote:
>>> Yes it is. But it the way to break integrity cos rows from table2 still
>>> refer to deleted rows from table1. So it conflicts with
>>> ideology isn't it?
>> Yes, but I'm not sure you could have a sensible behaviour-modifying BEFORE
>> trigger without this loophole. Don't forget, ordinary users can't work
>> around this - you need suitable permissions.
>>
>> You could rewrite PG's foreign-key code to check the referencing table after
>> the delete is supposed to have taken place, and make sure it has. That's
>> going to halve the speed of all your foreign-key checks though.
>>
>
> I'm not sure I've understood you right, sorry. Does "rewrite PG's
> foreign-key code" mean DDL? If it does how could I do this?

No, I was saying that to change this you'd have to alter PostgreSQL's
source-code.

You'd also have the issue of what to do with other triggers. You'd need
some priority level setting to allow some triggers to override other
triggers, but not the reverse.

If you really want to suppress deletion from table2 while enforcing
deletion via foreign-key you're best off with something like:

CREATE OR REPLACE FUNCTION fktrigfn() RETURNS TRIGGER AS $$
BEGIN
     PERFORM 1 FROM table1 WHERE a = OLD.aref;
     IF FOUND THEN
         RAISE NOTICE 'aborting delete for %', OLD.aref;
         RETURN NULL;
     ELSE
         RAISE NOTICE 'allowing delete for %', OLD.aref;
         RETURN OLD;
     END IF;
END;
$$ LANGUAGE plpgsql;

That should be OK, because the row should always be marked as removed
from table1 before the delete cascades.

--
   Richard Huxton
   Archonet Ltd

Re: Referential integrity vulnerability in 8.3.3

От
David Fetter
Дата:
On Tue, Jul 15, 2008 at 06:02:27PM +0400, Sergey Konoplev wrote:
> >>
> >> Yes it is. But it the way to break integrity cos rows from table2 still
> >> refer to deleted rows from table1. So it conflicts with
> >> ideology isn't it?
> >
> > Yes, but I'm not sure you could have a sensible behaviour-modifying BEFORE
> > trigger without this loophole. Don't forget, ordinary users can't work
> > around this - you need suitable permissions.
> >
> > You could rewrite PG's foreign-key code to check the referencing table after
> > the delete is supposed to have taken place, and make sure it has. That's
> > going to halve the speed of all your foreign-key checks though.
> >
>
> I'm not sure I've understood you right, sorry. Does "rewrite PG's
> foreign-key code" mean DDL? If it does how could I do this?

The code you posted is a clear case of doing things wrong
deliberately.  In order to prevent this error, you would need to
rewrite large parts of Postgres's code which checks referential
integrity, and there would still be things that deliberately wrong
DDL, triggers, rules, etc. could do.

Cheers,
David.
--
David Fetter <david@fetter.org> http://fetter.org/
Phone: +1 415 235 3778  AIM: dfetter666  Yahoo!: dfetter
Skype: davidfetter      XMPP: david.fetter@gmail.com

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate

Re: Referential integrity vulnerability in 8.3.3

От
"Sergey Konoplev"
Дата:
On Tue, Jul 15, 2008 at 7:17 PM, Richard Huxton <dev@archonet.com> wrote:
> Sergey Konoplev wrote:
>>>>
>>>> Yes it is. But it the way to break integrity cos rows from table2 still
>>>> refer to deleted rows from table1. So it conflicts with
>>>> ideology isn't it?
>>>
>>> Yes, but I'm not sure you could have a sensible behaviour-modifying
>>> BEFORE
>>> trigger without this loophole. Don't forget, ordinary users can't work
>>> around this - you need suitable permissions.
>>>
>>> You could rewrite PG's foreign-key code to check the referencing table
>>> after
>>> the delete is supposed to have taken place, and make sure it has. That's
>>> going to halve the speed of all your foreign-key checks though.
>>>
>>
>> I'm not sure I've understood you right, sorry. Does "rewrite PG's
>> foreign-key code" mean DDL? If it does how could I do this?
>
> No, I was saying that to change this you'd have to alter PostgreSQL's
> source-code.
>
> You'd also have the issue of what to do with other triggers. You'd need some
> priority level setting to allow some triggers to override other triggers,
> but not the reverse.
>
> If you really want to suppress deletion from table2 while enforcing deletion
> via foreign-key you're best off with something like:
>
> CREATE OR REPLACE FUNCTION fktrigfn() RETURNS TRIGGER AS $$
> BEGIN
>    PERFORM 1 FROM table1 WHERE a = OLD.aref;
>    IF FOUND THEN
>        RAISE NOTICE 'aborting delete for %', OLD.aref;
>        RETURN NULL;
>    ELSE
>        RAISE NOTICE 'allowing delete for %', OLD.aref;
>        RETURN OLD;
>    END IF;
> END;
> $$ LANGUAGE plpgsql;
>
> That should be OK, because the row should always be marked as removed from
> table1 before the delete cascades.

Well, your solution doesn't solve the main problem that sounds like
"Table2 contains rows with FK fields refer to deleted rows from table1
when ON DELETE action of the FKs is CASCADE". The only additional
thing fktrigfn() does is informing about "zombie" rows appearance in
logs.

--
Regards,
Sergey Konoplev

Re: Referential integrity vulnerability in 8.3.3

От
"Sergey Konoplev"
Дата:
> The code you posted is a clear case of doing things wrong
> deliberately.

Yes It's just an example. My real code is more complex of course.

> In order to prevent this error, you would need to
> rewrite large parts of Postgres's code which checks referential
> integrity, and there would still be things that deliberately wrong
> DDL, triggers, rules, etc. could do.

Sad to hear it. Anyway thanx for explanation.

--
Regards,
Sergey Konoplev

Re: Referential integrity vulnerability in 8.3.3

От
Klint Gore
Дата:
Sergey Konoplev wrote:
> > CREATE OR REPLACE FUNCTION fktrigfn() RETURNS TRIGGER AS $$
> > BEGIN
> >    PERFORM 1 FROM table1 WHERE a = OLD.aref;
> >    IF FOUND THEN
> >        RAISE NOTICE 'aborting delete for %', OLD.aref;
> >        RETURN NULL;
> >    ELSE
> >        RAISE NOTICE 'allowing delete for %', OLD.aref;
> >        RETURN OLD;
> >    END IF;
> > END;
> > $$ LANGUAGE plpgsql;
> >
> > That should be OK, because the row should always be marked as removed from
> > table1 before the delete cascades.
>
> Well, your solution doesn't solve the main problem that sounds like
> "Table2 contains rows with FK fields refer to deleted rows from table1
> when ON DELETE action of the FKs is CASCADE". The only additional
> thing fktrigfn() does is informing about "zombie" rows appearance in
> logs.
>
>
It does work around the problem.  The perform line sets found to true if
the row exists in the referred table and returns the NULL to prevent the
delete without crashing the transaction.  If it doesn't find the row in
the referred table, then it assumes it must be in a foreign key
cascading delete and returns OLD so that the rest of the delete happens.

i.e. the sequence of events is

1. statement delete from table1 where pk=blah
2. the row is removed from table1
3. attempt delete on table2
4. fktrigfn fires
5. found is set to false by the perform
6. old is returned
7. the row is removed from table2

as opposed to

1. statement delete from table2 where pk=foo
2. fktrigfn fires
3. found is set to true by the perform
4. null is returned
5. nothing changes

You would need to work the same logic into where you return null in your
real trigger.

klint.

--
Klint Gore
Database Manager
Sheep CRC
A.G.B.U.
University of New England
Armidale NSW 2350

Ph: 02 6773 3789
Fax: 02 6773 3266
EMail: kgore4@une.edu.au


Re: Referential integrity vulnerability in 8.3.3

От
Joris Dobbelsteen
Дата:
Richard Huxton wrote, On 15-Jul-2008 15:19:
> Sergey Konoplev wrote:
>> Yes it is. But it the way to break integrity cos rows from table2
>> still refer to deleted rows from table1. So it conflicts with
>> ideology isn't it?
>
> Yes, but I'm not sure you could have a sensible behaviour-modifying
> BEFORE trigger without this loophole. Don't forget, ordinary users can't
> work around this - you need suitable permissions.
 >
> You could rewrite PG's foreign-key code to check the referencing table
> after the delete is supposed to have taken place, and make sure it has.
> That's going to halve the speed of all your foreign-key checks though.

I did long ago.

For this to work you need to bypass the MVCC rules (to some extend). You
CANNOT do this with SQL statements, as there is no infrastructure for this.

For now you are bound to native foreign keys or triggers written in C
using (unsupported?) functions.

- Joris