From 4c3a5023f871a70549bce4a7d750231c3f54df76 Mon Sep 17 00:00:00 2001 From: Sergey Shinderuk Date: Thu, 4 Nov 2021 18:21:07 +0300 Subject: [PATCH] Guard against search-path based attacks. Use qualified references to functions and operators in the SQL queries executed by the event triggers to prevent users from defining their own functions or operators to replace them. This would not prevent audit logging, but it would allow the user to modify the type and name of the object in the DDL statement being audited. --- expected/pgaudit.out | 25 +++++++++++++++++++++++++ pgaudit.c | 14 ++++++++------ sql/pgaudit.sql | 32 ++++++++++++++++++++++++++++++++ 3 files changed, 65 insertions(+), 6 deletions(-) diff --git a/expected/pgaudit.out b/expected/pgaudit.out index 32afbca..7e5604d 100644 --- a/expected/pgaudit.out +++ b/expected/pgaudit.out @@ -2476,6 +2476,31 @@ NOTICE: AUDIT: OBJECT,1,1,READ,SELECT,TABLE,public.account,, (FUNCTION = my_ne, LEFTARG = text, RIGHTARG = text); +CREATE EXTENSION IF NOT EXISTS pgaudit; +SET pgaudit.log = 'DDL'; +-- Put public schema before pg_catalog to capture unqualified references +SET search_path = public, pg_catalog; +-- If there was a vulnerability, these would fail with division by zero error +CREATE TABLE wombat (); +NOTICE: AUDIT: SESSION,1,1,DDL,CREATE TABLE,TABLE,public.wombat,CREATE TABLE wombat ();, +DROP TABLE wombat; +NOTICE: AUDIT: SESSION,2,1,DDL,DROP TABLE,TABLE,public.wombat,DROP TABLE wombat;, +SET pgaudit.log = 'NONE'; +DROP EXTENSION pgaudit; +DROP OPERATOR <> (text, text); +DROP FUNCTION my_ne(text, text); +DROP FUNCTION lower(text); +DROP FUNCTION upper(text); -- Cleanup -- Set client_min_messages up to warning to avoid noise SET client_min_messages = 'warning'; diff --git a/pgaudit.c b/pgaudit.c index 275340c..747238d 100644 --- a/pgaudit.c +++ b/pgaudit.c @@ -1655,7 +1655,9 @@ pgaudit_ddl_command_end(PG_FUNCTION_ARGS) CreateCommandTag(eventData->parsetree); /* Return objects affected by the (non drop) DDL statement */ - query = "SELECT UPPER(object_type), object_identity, UPPER(command_tag)\n" + query = "SELECT pg_catalog.upper(object_type),\n" + " object_identity,\n" + " pg_catalog.upper(command_tag)\n" " FROM pg_catalog.pg_event_trigger_ddl_commands()"; /* Attempt to connect */ @@ -1754,11 +1756,11 @@ pgaudit_sql_drop(PG_FUNCTION_ARGS) contextOld = MemoryContextSwitchTo(contextQuery); /* Return objects affected by the drop statement */ - query = "SELECT UPPER(object_type),\n" - " object_identity\n" - " FROM pg_catalog.pg_event_trigger_dropped_objects()\n" - " WHERE lower(object_type) <> 'type'\n" - " AND schema_name <> 'pg_toast'"; + query = "SELECT pg_catalog.upper(object_type),\n" + " object_identity\n" + " FROM pg_catalog.pg_event_trigger_dropped_objects()\n" + " WHERE pg_catalog.lower(object_type) operator(pg_catalog.<>) 'type'\n" + " AND schema_name operator(pg_catalog.<>) 'pg_toast'"; /* Attempt to connect */ result = SPI_connect(); diff --git a/sql/pgaudit.sql b/sql/pgaudit.sql index dda81b0..61c728e 100644 --- a/sql/pgaudit.sql +++ b/sql/pgaudit.sql @@ -1545,6 +1545,38 @@ SELECT * FROM account; -- Change back to superuser to do exhaustive tests \connect - :current_user +-- +-- Test that pgaudit event triggers are immune to search-path-based attacks + +-- Attempt to capture unqualified references to standard functions +CREATE FUNCTION upper(text) RETURNS text +LANGUAGE SQL AS 'SELECT (1/0)::text'; + +CREATE FUNCTION lower(text) RETURNS text +LANGUAGE SQL AS 'SELECT (1/0)::text'; + +CREATE FUNCTION my_ne(text, text) RETURNS bool +LANGUAGE SQL AS 'SELECT (1/0)::bool'; + +CREATE OPERATOR <> (FUNCTION = my_ne, LEFTARG = text, RIGHTARG = text); + +CREATE EXTENSION IF NOT EXISTS pgaudit; +SET pgaudit.log = 'DDL'; +-- Put public schema before pg_catalog to capture unqualified references +SET search_path = public, pg_catalog; + +-- If there was a vulnerability, these would fail with division by zero error +CREATE TABLE wombat (); +DROP TABLE wombat; + +SET pgaudit.log = 'NONE'; +DROP EXTENSION pgaudit; + +DROP OPERATOR <> (text, text); +DROP FUNCTION my_ne(text, text); +DROP FUNCTION lower(text); +DROP FUNCTION upper(text); + -- Cleanup -- Set client_min_messages up to warning to avoid noise SET client_min_messages = 'warning';