1

I want to keep all changes of my tables. I have a working solution for making a trigger per table, but it seems silly to copy the code foreach table. Is there any way to create a single trigger function that does this?

Example of my working per-table trigger (including table definitions):

CREATE TABLE departments (
    id                  bigserial Primary Key,
    name                varchar not null,
    created             bigint not null default date_part('epoch', NOW()),
    created_by          bigint references Employees (id) not null
);
create table Departments_hist ("action" varchar not null, change_date bigint not null, rev bigserial not null, like Departments);
CREATE OR REPLACE FUNCTION add_to_history_Departments() RETURNS TRIGGER AS $$
BEGIN
IF(TG_OP='INSERT' OR TG_OP='UPDATE') THEN
    INSERT INTO Departments_hist values (TG_OP,date_part('epoch', NOW()),DEFAULT,NEW.*);
END IF;
IF (TG_OP='DELETE') THEN
    INSERT INTO Departments_hist values (TG_OP,date_part('epoch', NOW()),DEFAULT,OLD.*);
END IF;
RETURN NEW;
END;
$$ LANGUAGE plpgsql;
CREATE TRIGGER trigger_history_Departments AFTER INSERT OR UPDATE OR DELETE ON Departments FOR EACH ROW EXECUTE PROCEDURE add_to_history_Departments();

I've tried to make it multi-table by concatenating '_hist' to TG_TABLE_NAME:

CREATE OR REPLACE FUNCTION add_to_hist_table() RETURNS TRIGGER AS $$
DECLARE
    histTable text :=TG_TABLE_NAME || '_hist';
BEGIN
    IF (TG_OP='INSERT' OR TG_OP='UPDATE') THEN
        INSERT INTO histTable values (TG_OP,date_part('epoch', NOW()),DEFAULT,NEW.*);
    ELSIF TG_OP='DELETE' THEN
        INSERT INTO histTable values (TG_OP,date_part('epoch', NOW()),DEFAULT,OLD.*);
    END IF;
    RETURN null; --ignored since it is an AFTER triggger.
END;
$$ LANGUAGE plpgsql;

But I get an error:

ERROR:  syntax error at or near "$1"
LINE 1: INSERT INTO  $1  values ( $2 ,date_part('epoch', NOW()),DEFA...
                     ^
QUERY:  INSERT INTO  $1  values ( $2 ,date_part('epoch', NOW()),DEFAULT, $3 .*)
CONTEXT:  SQL statement in PL/PgSQL function "add_to_hist_table" near line 5

I guess it is a problem with variable substitution ( http://www.postgresql.org/docs/8.4/static/plpgsql-implementation.html ).

How can this functionality be achieved?

PS. I'm using postgresql 8.4 but will likely upgrade to 9.3 soon.

Pascal_dher
  • 448
  • 3
  • 14

2 Answers2

1

I found a solution on this "related question" https://stackoverflow.com/a/1997417/844731

I didn't think of doing 'EXECUTE USING' with NEW and OLD. So now a working solution is:

CREATE OR REPLACE FUNCTION add_to_hist_table() RETURNS TRIGGER AS $$
BEGIN
IF (TG_OP='INSERT' OR TG_OP='UPDATE') THEN
    execute 'INSERT INTO '|| TG_TABLE_NAME ||'_hist values (''' || TG_OP || ''',date_part(''epoch'', NOW()),DEFAULT,$1.*)' using NEW;
ELSIF TG_OP='DELETE' THEN
    execute 'INSERT INTO '|| TG_TABLE_NAME ||'_hist values (''' || TG_OP || ''',date_part(''epoch'', NOW()),DEFAULT,$1.*)'  using OLD;
END IF;
RETURN null; --ignored since it is an AFTER triggger.
END;
$$ LANGUAGE plpgsql;
Community
  • 1
  • 1
Pascal_dher
  • 448
  • 3
  • 14
  • attention - this example is vulnerable to SQL injection. Any parameter is dynamic query string must be sanitized (escaped). – Pavel Stehule Jan 31 '16 at 11:06
  • hi @PavelStehule. How or where is it vurnable? ( It is a trigger and I don't use any user-input variables that have not already been interpreted by postgres) – Pascal_dher Feb 01 '16 at 08:32
  • @PavelStehule could you please point out where it is vurnable or demonstrate a better solution? Thanks :-) – Pascal_dher Feb 01 '16 at 22:07
0

@Pascal_dher, somebody can create table with name with containing attacker code. Due Postgresql this probably dosnt do some really bad, only failed queries. But if your trigger will be more complex, then impacts can be much more worse.

Pavel Stehule
  • 42,331
  • 5
  • 91
  • 94
  • but if they can create tables before exploiting can't they already do everything? Would using quote_ident (tg_table_name||'_hist') and quote_literal(tg_op) fix potential sql injection? – Pascal_dher Feb 03 '16 at 09:03