Fix use-after-free bug with event triggers in an extension script

ALTER TABLE commands in an extension script are added to an event
trigger command list; but starting with commit b5810de3f4 they do so in
a memory context that's too short-lived, so when execution ends and time
comes to use the entries, they've already been freed.

(This would also be a problem with ALTER TABLE commands in a
multi-command query string, but these serendipitously end in
PortalContext -- which probably explains why it took so long for this to
be reported.)

Fix by using the memory context specifically set for that, instead.

Backpatch to 13, where the aforementioned commit appeared.

Reported-by: Philippe Beaudoin
Author: Jehan-Guillaume de Rorthais <[email protected]>
Discussion: https://postgr.es/m/20200902193715.6e0269d4@firost

diff --git a/src/backend/commands/event_trigger.c b/src/backend/commands/event_trigger.c
index 7844880170aebbaff93f4984526fe8c083d9144f..8bb17c34f5b06271927923bba86cf5203dccbaaf 100644 (file)
--- a/src/backend/commands/event_trigger.c
+++ b/src/backend/commands/event_trigger.c
@@ -1646,9 +1646,15 @@ EventTriggerAlterTableEnd(void)
    /* If no subcommands, don't collect */
    if (list_length(currentEventTriggerState->currentCommand->d.alterTable.subcmds) != 0)
    {
+       MemoryContext oldcxt;
+
+       oldcxt = MemoryContextSwitchTo(currentEventTriggerState->cxt);
+
        currentEventTriggerState->commandList =
            lappend(currentEventTriggerState->commandList,
                    currentEventTriggerState->currentCommand);
+
+       MemoryContextSwitchTo(oldcxt);
    }
    else
        pfree(currentEventTriggerState->currentCommand);

diff --git a/src/test/modules/test_extensions/Makefile b/src/test/modules/test_extensions/Makefile
index d18108e4e5abaa660e99936c8b249c8a955c5780..77ee4d5d9ecd5016cf59853b33b8471bbee1de75 100644 (file)
--- a/src/test/modules/test_extensions/Makefile
+++ b/src/test/modules/test_extensions/Makefile
@@ -4,11 +4,13 @@ MODULE = test_extensions
 PGFILEDESC = "test_extensions - regression testing for EXTENSION support"
 
 EXTENSION = test_ext1 test_ext2 test_ext3 test_ext4 test_ext5 test_ext6 \
-            test_ext7 test_ext8 test_ext_cyclic1 test_ext_cyclic2
+            test_ext7 test_ext8 test_ext_cyclic1 test_ext_cyclic2 \
+            test_ext_evttrig
 DATA = test_ext1--1.0.sql test_ext2--1.0.sql test_ext3--1.0.sql \
        test_ext4--1.0.sql test_ext5--1.0.sql test_ext6--1.0.sql \
        test_ext7--1.0.sql test_ext7--1.0--2.0.sql test_ext8--1.0.sql \
-       test_ext_cyclic1--1.0.sql test_ext_cyclic2--1.0.sql
+       test_ext_cyclic1--1.0.sql test_ext_cyclic2--1.0.sql \
+       test_ext_evttrig--1.0.sql test_ext_evttrig--1.0--2.0.sql
 
 REGRESS = test_extensions test_extdepend
 

diff --git a/src/test/modules/test_extensions/expected/test_extensions.out b/src/test/modules/test_extensions/expected/test_extensions.out
index b5cbdfcad4f337c820c34e864fb662d5bb6ea52b..30ae621d05ef6d013dd649f83a876ebb0bd5ba6c 100644 (file)
--- a/src/test/modules/test_extensions/expected/test_extensions.out
+++ b/src/test/modules/test_extensions/expected/test_extensions.out
@@ -154,3 +154,8 @@ DROP TABLE test_ext4_tab;
 DROP FUNCTION create_extension_with_temp_schema();
 RESET client_min_messages;
 \unset SHOW_CONTEXT
+-- Test case of an event trigger run in an extension upgrade script.
+-- See: https://postgr.es/m/20200902193715.6e0269d4@firost
+CREATE EXTENSION test_ext_evttrig;
+ALTER EXTENSION test_ext_evttrig UPDATE TO '2.0';
+DROP EXTENSION test_ext_evttrig;

diff --git a/src/test/modules/test_extensions/sql/test_extensions.sql b/src/test/modules/test_extensions/sql/test_extensions.sql
index f505466ab4ebbf00034c8b6836f85eee8c90a4b2..c16fd36da8967482e623b8200fd9cabdeb919804 100644 (file)
--- a/src/test/modules/test_extensions/sql/test_extensions.sql
+++ b/src/test/modules/test_extensions/sql/test_extensions.sql
@@ -93,3 +93,9 @@ DROP TABLE test_ext4_tab;
 DROP FUNCTION create_extension_with_temp_schema();
 RESET client_min_messages;
 \unset SHOW_CONTEXT
+
+-- Test case of an event trigger run in an extension upgrade script.
+-- See: https://postgr.es/m/20200902193715.6e0269d4@firost
+CREATE EXTENSION test_ext_evttrig;
+ALTER EXTENSION test_ext_evttrig UPDATE TO '2.0';
+DROP EXTENSION test_ext_evttrig;

diff --git a/src/test/modules/test_extensions/test_ext_evttrig--1.0--2.0.sql b/src/test/modules/test_extensions/test_ext_evttrig--1.0--2.0.sql
new file mode 100644 (file)
index 0000000..fdd2f35
--- /dev/null
+++ b/src/test/modules/test_extensions/test_ext_evttrig--1.0--2.0.sql
@@ -0,0 +1,7 @@
+/* src/test/modules/test_extensions/test_event_trigger--1.0--2.0.sql */
+-- complain if script is sourced in psql, rather than via ALTER EXTENSION
+\echo Use "ALTER EXTENSION test_event_trigger UPDATE TO '2.0'" to load this file. \quit
+
+-- Test extension upgrade with event trigger.
+ALTER EVENT TRIGGER table_rewrite_trg DISABLE;
+ALTER TABLE t DROP COLUMN id;

diff --git a/src/test/modules/test_extensions/test_ext_evttrig--1.0.sql b/src/test/modules/test_extensions/test_ext_evttrig--1.0.sql
new file mode 100644 (file)
index 0000000..0071712
--- /dev/null
+++ b/src/test/modules/test_extensions/test_ext_evttrig--1.0.sql
@@ -0,0 +1,16 @@
+/* src/test/modules/test_extensions/test_event_trigger--1.0.sql */
+-- complain if script is sourced in psql, rather than via CREATE EXTENSION
+\echo Use "CREATE EXTENSION test_event_trigger" to load this file. \quit
+
+-- Base table with event trigger, used in a regression test involving
+-- extension upgrades.
+CREATE TABLE t (id text);
+CREATE OR REPLACE FUNCTION _evt_table_rewrite_fnct()
+RETURNS EVENT_TRIGGER LANGUAGE plpgsql AS
+$$
+  BEGIN
+  END;
+$$;
+CREATE EVENT TRIGGER table_rewrite_trg
+  ON table_rewrite
+  EXECUTE PROCEDURE _evt_table_rewrite_fnct();

diff --git a/src/test/modules/test_extensions/test_ext_evttrig.control b/src/test/modules/test_extensions/test_ext_evttrig.control
new file mode 100644 (file)
index 0000000..915fae6
--- /dev/null
+++ b/src/test/modules/test_extensions/test_ext_evttrig.control
@@ -0,0 +1,3 @@
+comment = 'Test extension - event trigger'
+default_version = '1.0'
+relocatable = true