Fix search_path to a safe value during maintenance operations.

While executing maintenance operations (ANALYZE, CLUSTER, REFRESH
MATERIALIZED VIEW, REINDEX, or VACUUM), set search_path to
'pg_catalog, pg_temp' to prevent inconsistent behavior.

Functions that are used for functional indexes, in index expressions,
or in materialized views and depend on a different search path must be
declared with CREATE FUNCTION ... SET search_path='...'.

This change addresses a security risk introduced in commit 60684dd834,
where a role with MAINTAIN privileges on a table may be able to
escalate privileges to the table owner. That commit is not yet part of
any release, so no need to backpatch.

Discussion: https://postgr.es/m/e44327179e5c9015c8dda67351c04da552066017.camel%40j-davis.com
Reviewed-by: Greg Stark
Reviewed-by: Nathan Bossart

diff --git a/contrib/amcheck/verify_nbtree.c b/contrib/amcheck/verify_nbtree.c
index 6979aff727be0dfd604183ec43f62545a6b4db90..2b5c8a300b06ecddbbf2259ea684c30eb7cc0512 100644 (file)
--- a/contrib/amcheck/verify_nbtree.c
+++ b/contrib/amcheck/verify_nbtree.c
@@ -282,6 +282,8 @@ bt_index_check_internal(Oid indrelid, bool parentcheck, bool heapallindexed,
        SetUserIdAndSecContext(heaprel->rd_rel->relowner,
                               save_sec_context | SECURITY_RESTRICTED_OPERATION);
        save_nestlevel = NewGUCNestLevel();
+       SetConfigOption("search_path", GUC_SAFE_SEARCH_PATH, PGC_USERSET,
+                       PGC_S_SESSION);
    }
    else
    {

diff --git a/src/backend/access/brin/brin.c b/src/backend/access/brin/brin.c
index 3c6a956eaa3f2c9dfb016e23a98b0667259bddb5..11e78cb62c797ce1c216694f1563d329a9842088 100644 (file)
--- a/src/backend/access/brin/brin.c
+++ b/src/backend/access/brin/brin.c
@@ -1066,6 +1066,8 @@ brin_summarize_range(PG_FUNCTION_ARGS)
        SetUserIdAndSecContext(heapRel->rd_rel->relowner,
                               save_sec_context | SECURITY_RESTRICTED_OPERATION);
        save_nestlevel = NewGUCNestLevel();
+       SetConfigOption("search_path", GUC_SAFE_SEARCH_PATH, PGC_USERSET,
+                       PGC_S_SESSION);
    }
    else
    {

diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
index 352e43d0e6133f9838cf8cedd7c1a5f77b0769fd..e8337041ac9a2d4e5876e00190981f48e0c0275f 100644 (file)
--- a/src/backend/catalog/index.c
+++ b/src/backend/catalog/index.c
@@ -1475,6 +1475,8 @@ index_concurrently_build(Oid heapRelationId,
    SetUserIdAndSecContext(heapRel->rd_rel->relowner,
                           save_sec_context | SECURITY_RESTRICTED_OPERATION);
    save_nestlevel = NewGUCNestLevel();
+   SetConfigOption("search_path", GUC_SAFE_SEARCH_PATH, PGC_USERSET,
+                   PGC_S_SESSION);
 
    indexRelation = index_open(indexRelationId, RowExclusiveLock);
 
@@ -3006,6 +3008,8 @@ index_build(Relation heapRelation,
    SetUserIdAndSecContext(heapRelation->rd_rel->relowner,
                           save_sec_context | SECURITY_RESTRICTED_OPERATION);
    save_nestlevel = NewGUCNestLevel();
+   SetConfigOption("search_path", GUC_SAFE_SEARCH_PATH, PGC_USERSET,
+                   PGC_S_SESSION);
 
    /* Set up initial progress report status */
    {
@@ -3341,6 +3345,8 @@ validate_index(Oid heapId, Oid indexId, Snapshot snapshot)
    SetUserIdAndSecContext(heapRelation->rd_rel->relowner,
                           save_sec_context | SECURITY_RESTRICTED_OPERATION);
    save_nestlevel = NewGUCNestLevel();
+   SetConfigOption("search_path", GUC_SAFE_SEARCH_PATH, PGC_USERSET,
+                   PGC_S_SESSION);
 
    indexRelation = index_open(indexId, RowExclusiveLock);
 
@@ -3601,6 +3607,8 @@ reindex_index(Oid indexId, bool skip_constraint_checks, char persistence,
    SetUserIdAndSecContext(heapRelation->rd_rel->relowner,
                           save_sec_context | SECURITY_RESTRICTED_OPERATION);
    save_nestlevel = NewGUCNestLevel();
+   SetConfigOption("search_path", GUC_SAFE_SEARCH_PATH, PGC_USERSET,
+                   PGC_S_SESSION);
 
    if (progress)
    {

diff --git a/src/backend/commands/analyze.c b/src/backend/commands/analyze.c
index 52ef462dba3fe92f8cf0c5d6f8f42db43231e487..2fdd1804976072b1a4a388e007befcdb50e0ac99 100644 (file)
--- a/src/backend/commands/analyze.c
+++ b/src/backend/commands/analyze.c
@@ -348,6 +348,8 @@ do_analyze_rel(Relation onerel, VacuumParams *params,
    SetUserIdAndSecContext(onerel->rd_rel->relowner,
                           save_sec_context | SECURITY_RESTRICTED_OPERATION);
    save_nestlevel = NewGUCNestLevel();
+   SetConfigOption("search_path", GUC_SAFE_SEARCH_PATH, PGC_USERSET,
+                   PGC_S_SESSION);
 
    /* measure elapsed time iff autovacuum logging requires it */
    if (IsAutoVacuumWorkerProcess() && params->log_min_duration >= 0)

diff --git a/src/backend/commands/cluster.c b/src/backend/commands/cluster.c
index 369fea7c046d9972833300a72dd9c9af68b865e5..c94a3e20d610994501e65dc4cc8458fb83e78c2d 100644 (file)
--- a/src/backend/commands/cluster.c
+++ b/src/backend/commands/cluster.c
@@ -355,6 +355,8 @@ cluster_rel(Oid tableOid, Oid indexOid, ClusterParams *params)
    SetUserIdAndSecContext(OldHeap->rd_rel->relowner,
                           save_sec_context | SECURITY_RESTRICTED_OPERATION);
    save_nestlevel = NewGUCNestLevel();
+   SetConfigOption("search_path", GUC_SAFE_SEARCH_PATH, PGC_USERSET,
+                   PGC_S_SESSION);
 
    /*
     * Since we may open a new transaction for each relation, we have to check

diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index a5168c9f097756ae74a889abdda354bb8b1a1208..a7c6a3dc7adb0b5dffa10bea1d65e314b6d2ac68 100644 (file)
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -575,6 +575,8 @@ DefineIndex(Oid relationId,
    int         root_save_nestlevel;
 
    root_save_nestlevel = NewGUCNestLevel();
+   SetConfigOption("search_path", GUC_SAFE_SEARCH_PATH, PGC_USERSET,
+                   PGC_S_SESSION);
 
    /*
     * Some callers need us to run with an empty default_tablespace; this is a
@@ -1300,6 +1302,8 @@ DefineIndex(Oid relationId,
                SetUserIdAndSecContext(childrel->rd_rel->relowner,
                                       child_save_sec_context | SECURITY_RESTRICTED_OPERATION);
                child_save_nestlevel = NewGUCNestLevel();
+               SetConfigOption("search_path", GUC_SAFE_SEARCH_PATH, PGC_USERSET,
+                               PGC_S_SESSION);
 
                /*
                 * Don't try to create indexes on foreign tables, though. Skip
@@ -3753,6 +3757,8 @@ ReindexRelationConcurrently(Oid relationOid, ReindexParams *params)
        SetUserIdAndSecContext(heapRel->rd_rel->relowner,
                               save_sec_context | SECURITY_RESTRICTED_OPERATION);
        save_nestlevel = NewGUCNestLevel();
+       SetConfigOption("search_path", GUC_SAFE_SEARCH_PATH, PGC_USERSET,
+                       PGC_S_SESSION);
 
        /* determine safety of this index for set_indexsafe_procflags */
        idx->safe = (indexRel->rd_indexprs == NIL &&

diff --git a/src/backend/commands/matview.c b/src/backend/commands/matview.c
index f9a3bdfc3abbff2bc2ad5d78252d5f91f3967a67..9114a7924c84a88b9acb8419e80e42dfc277b0aa 100644 (file)
--- a/src/backend/commands/matview.c
+++ b/src/backend/commands/matview.c
@@ -179,6 +179,8 @@ ExecRefreshMatView(RefreshMatViewStmt *stmt, const char *queryString,
    SetUserIdAndSecContext(relowner,
                           save_sec_context | SECURITY_RESTRICTED_OPERATION);
    save_nestlevel = NewGUCNestLevel();
+   SetConfigOption("search_path", GUC_SAFE_SEARCH_PATH, PGC_USERSET,
+                   PGC_S_SESSION);
 
    /* Make sure it is a materialized view. */
    if (matviewRel->rd_rel->relkind != RELKIND_MATVIEW)

diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c
index a843f9ad926e0a893d7b8372aa0b02e817c9c889..c08261179739e3381451beb230ae7301382d872f 100644 (file)
--- a/src/backend/commands/vacuum.c
+++ b/src/backend/commands/vacuum.c
@@ -2172,6 +2172,8 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params,
    SetUserIdAndSecContext(rel->rd_rel->relowner,
                           save_sec_context | SECURITY_RESTRICTED_OPERATION);
    save_nestlevel = NewGUCNestLevel();
+   SetConfigOption("search_path", GUC_SAFE_SEARCH_PATH, PGC_USERSET,
+                   PGC_S_SESSION);
 
    /*
     * If PROCESS_MAIN is set (the default), it's time to vacuum the main

diff --git a/src/bin/scripts/t/100_vacuumdb.pl b/src/bin/scripts/t/100_vacuumdb.pl
index eccfcc54a1a196d3a146d42b5baad271f6d39c14..f91b5127a83847262f457a40d36ff68d7b17f2dd 100644 (file)
--- a/src/bin/scripts/t/100_vacuumdb.pl
+++ b/src/bin/scripts/t/100_vacuumdb.pl
@@ -109,15 +109,11 @@ $node->safe_psql(
   CREATE FUNCTION f1(int) RETURNS int LANGUAGE SQL AS 'SELECT f0($1)';
   CREATE TABLE funcidx (x int);
   INSERT INTO funcidx VALUES (0),(1),(2),(3);
-  CREATE INDEX i0 ON funcidx ((f1(x)));
   CREATE SCHEMA "Foo";
   CREATE TABLE "Foo".bar(id int);
 |);
 $node->command_ok([qw|vacuumdb -Z --table="need""q(uot"(")x") postgres|],
    'column list');
-$node->command_fails(
-   [qw|vacuumdb -Zt funcidx postgres|],
-   'unqualified name via functional index');
 
 $node->command_fails(
    [ 'vacuumdb', '--analyze', '--table', 'vactable(c)', 'postgres' ],

diff --git a/src/include/utils/guc.h b/src/include/utils/guc.h
index d5253c7ed23d23db6031a62e3c59e7a71893d169..edd82a551ba7c769515b558dd2f58c193be3382d 100644 (file)
--- a/src/include/utils/guc.h
+++ b/src/include/utils/guc.h
@@ -201,6 +201,12 @@ typedef enum
 
 #define GUC_QUALIFIER_SEPARATOR '.'
 
+/*
+ * Safe search path when executing code as the table owner, such as during
+ * maintenance operations.
+ */
+#define GUC_SAFE_SEARCH_PATH "pg_catalog, pg_temp"
+
 /*
  * Bit values in "flags" of a GUC variable.  Note that these don't appear
  * on disk, so we can reassign their values freely.

diff --git a/src/test/modules/test_oat_hooks/expected/test_oat_hooks.out b/src/test/modules/test_oat_hooks/expected/test_oat_hooks.out
index f80373aecc03351dd127e43c15bbfe1f7e0ad9c5..effdc4914583a0c81da642464d6f78d99fed992f 100644 (file)
--- a/src/test/modules/test_oat_hooks/expected/test_oat_hooks.out
+++ b/src/test/modules/test_oat_hooks/expected/test_oat_hooks.out
@@ -89,11 +89,15 @@ NOTICE:  in object access: superuser finished create (subId=0x0) [internal]
 NOTICE:  in process utility: superuser finished CREATE TABLE
 CREATE INDEX regress_test_table_t_idx ON regress_test_table (t);
 NOTICE:  in process utility: superuser attempting CREATE INDEX
+NOTICE:  in object access: superuser attempting namespace search (subId=0x0) [no report on violation, allowed]
+NOTICE:  in object access: superuser finished namespace search (subId=0x0) [no report on violation, allowed]
 NOTICE:  in object access: superuser attempting create (subId=0x0) [explicit]
 NOTICE:  in object access: superuser finished create (subId=0x0) [explicit]
 NOTICE:  in process utility: superuser finished CREATE INDEX
 GRANT SELECT ON Table regress_test_table TO public;
 NOTICE:  in process utility: superuser attempting GRANT
+NOTICE:  in object access: superuser attempting namespace search (subId=0x0) [no report on violation, allowed]
+NOTICE:  in object access: superuser finished namespace search (subId=0x0) [no report on violation, allowed]
 NOTICE:  in process utility: superuser finished GRANT
 CREATE FUNCTION regress_test_func (t text) RETURNS text AS $$
    SELECT $1;

diff --git a/src/test/regress/expected/privileges.out b/src/test/regress/expected/privileges.out
index 3cf4ac8c9ed257e7852ceb96b2caf015bacf28f2..997c5c57a5ce5dd226a1eae6a238fbba4b2fe971 100644 (file)
--- a/src/test/regress/expected/privileges.out
+++ b/src/test/regress/expected/privileges.out
@@ -1769,7 +1769,7 @@ SET SESSION AUTHORIZATION regress_sro_user;
 CREATE FUNCTION unwanted_grant() RETURNS void LANGUAGE sql AS
    'GRANT regress_priv_group2 TO regress_sro_user';
 CREATE FUNCTION mv_action() RETURNS bool LANGUAGE sql AS
-   'DECLARE c CURSOR WITH HOLD FOR SELECT unwanted_grant(); SELECT true';
+   'DECLARE c CURSOR WITH HOLD FOR SELECT public.unwanted_grant(); SELECT true';
 -- REFRESH of this MV will queue a GRANT at end of transaction
 CREATE MATERIALIZED VIEW sro_mv AS SELECT mv_action() WITH NO DATA;
 REFRESH MATERIALIZED VIEW sro_mv;
@@ -1783,12 +1783,12 @@ SET SESSION AUTHORIZATION regress_sro_user;
 -- INSERT to this table will queue a GRANT at end of transaction
 CREATE TABLE sro_trojan_table ();
 CREATE FUNCTION sro_trojan() RETURNS trigger LANGUAGE plpgsql AS
-   'BEGIN PERFORM unwanted_grant(); RETURN NULL; END';
+   'BEGIN PERFORM public.unwanted_grant(); RETURN NULL; END';
 CREATE CONSTRAINT TRIGGER t AFTER INSERT ON sro_trojan_table
     INITIALLY DEFERRED FOR EACH ROW EXECUTE PROCEDURE sro_trojan();
 -- Now, REFRESH will issue such an INSERT, queueing the GRANT
 CREATE OR REPLACE FUNCTION mv_action() RETURNS bool LANGUAGE sql AS
-   'INSERT INTO sro_trojan_table DEFAULT VALUES; SELECT true';
+   'INSERT INTO public.sro_trojan_table DEFAULT VALUES; SELECT true';
 REFRESH MATERIALIZED VIEW sro_mv;
 ERROR:  cannot fire deferred trigger within security-restricted operation
 CONTEXT:  SQL function "mv_action" statement 1
@@ -1800,15 +1800,15 @@ BEGIN; SET CONSTRAINTS ALL IMMEDIATE; REFRESH MATERIALIZED VIEW sro_mv; COMMIT;
 ERROR:  permission denied to grant role "regress_priv_group2"
 DETAIL:  Only roles with the ADMIN option on role "regress_priv_group2" may grant this role.
 CONTEXT:  SQL function "unwanted_grant" statement 1
-SQL statement "SELECT unwanted_grant()"
-PL/pgSQL function sro_trojan() line 1 at PERFORM
+SQL statement "SELECT public.unwanted_grant()"
+PL/pgSQL function public.sro_trojan() line 1 at PERFORM
 SQL function "mv_action" statement 1
 -- REFRESH MATERIALIZED VIEW CONCURRENTLY use of eval_const_expressions()
 SET SESSION AUTHORIZATION regress_sro_user;
 CREATE FUNCTION unwanted_grant_nofail(int) RETURNS int
    IMMUTABLE LANGUAGE plpgsql AS $$
 BEGIN
-   PERFORM unwanted_grant();
+   PERFORM public.unwanted_grant();
    RAISE WARNING 'owned';
    RETURN 1;
 EXCEPTION WHEN OTHERS THEN

diff --git a/src/test/regress/expected/vacuum.out b/src/test/regress/expected/vacuum.out
index 41e020cf20c9aaca1569fc0a7369291a7065aa14..454821c16b52b441383574793467ddea4c6320d2 100644 (file)
--- a/src/test/regress/expected/vacuum.out
+++ b/src/test/regress/expected/vacuum.out
@@ -64,7 +64,7 @@ CLUSTER vaccluster;
 CREATE FUNCTION do_analyze() RETURNS VOID VOLATILE LANGUAGE SQL
    AS 'ANALYZE pg_am';
 CREATE FUNCTION wrap_do_analyze(c INT) RETURNS INT IMMUTABLE LANGUAGE SQL
-   AS 'SELECT $1 FROM do_analyze()';
+   AS 'SELECT $1 FROM public.do_analyze()';
 CREATE INDEX ON vaccluster(wrap_do_analyze(i));
 INSERT INTO vaccluster VALUES (1), (2);
 ANALYZE vaccluster;

diff --git a/src/test/regress/sql/privileges.sql b/src/test/regress/sql/privileges.sql
index 134809e8cc559c902bebd1177030f428e2366d1f..e961748d790fbada504cb1a6274ecc7d602c0b16 100644 (file)
--- a/src/test/regress/sql/privileges.sql
+++ b/src/test/regress/sql/privileges.sql
@@ -1177,7 +1177,7 @@ SET SESSION AUTHORIZATION regress_sro_user;
 CREATE FUNCTION unwanted_grant() RETURNS void LANGUAGE sql AS
    'GRANT regress_priv_group2 TO regress_sro_user';
 CREATE FUNCTION mv_action() RETURNS bool LANGUAGE sql AS
-   'DECLARE c CURSOR WITH HOLD FOR SELECT unwanted_grant(); SELECT true';
+   'DECLARE c CURSOR WITH HOLD FOR SELECT public.unwanted_grant(); SELECT true';
 -- REFRESH of this MV will queue a GRANT at end of transaction
 CREATE MATERIALIZED VIEW sro_mv AS SELECT mv_action() WITH NO DATA;
 REFRESH MATERIALIZED VIEW sro_mv;
@@ -1188,12 +1188,12 @@ SET SESSION AUTHORIZATION regress_sro_user;
 -- INSERT to this table will queue a GRANT at end of transaction
 CREATE TABLE sro_trojan_table ();
 CREATE FUNCTION sro_trojan() RETURNS trigger LANGUAGE plpgsql AS
-   'BEGIN PERFORM unwanted_grant(); RETURN NULL; END';
+   'BEGIN PERFORM public.unwanted_grant(); RETURN NULL; END';
 CREATE CONSTRAINT TRIGGER t AFTER INSERT ON sro_trojan_table
     INITIALLY DEFERRED FOR EACH ROW EXECUTE PROCEDURE sro_trojan();
 -- Now, REFRESH will issue such an INSERT, queueing the GRANT
 CREATE OR REPLACE FUNCTION mv_action() RETURNS bool LANGUAGE sql AS
-   'INSERT INTO sro_trojan_table DEFAULT VALUES; SELECT true';
+   'INSERT INTO public.sro_trojan_table DEFAULT VALUES; SELECT true';
 REFRESH MATERIALIZED VIEW sro_mv;
 \c -
 REFRESH MATERIALIZED VIEW sro_mv;
@@ -1204,7 +1204,7 @@ SET SESSION AUTHORIZATION regress_sro_user;
 CREATE FUNCTION unwanted_grant_nofail(int) RETURNS int
    IMMUTABLE LANGUAGE plpgsql AS $$
 BEGIN
-   PERFORM unwanted_grant();
+   PERFORM public.unwanted_grant();
    RAISE WARNING 'owned';
    RETURN 1;
 EXCEPTION WHEN OTHERS THEN

diff --git a/src/test/regress/sql/vacuum.sql b/src/test/regress/sql/vacuum.sql
index 51d7b1fecc71044fd66a27a19f99ebfe8a9125df..0b63ef8dc664cac863486d2f484f630e5c5a88f3 100644 (file)
--- a/src/test/regress/sql/vacuum.sql
+++ b/src/test/regress/sql/vacuum.sql
@@ -49,7 +49,7 @@ CLUSTER vaccluster;
 CREATE FUNCTION do_analyze() RETURNS VOID VOLATILE LANGUAGE SQL
    AS 'ANALYZE pg_am';
 CREATE FUNCTION wrap_do_analyze(c INT) RETURNS INT IMMUTABLE LANGUAGE SQL
-   AS 'SELECT $1 FROM do_analyze()';
+   AS 'SELECT $1 FROM public.do_analyze()';
 CREATE INDEX ON vaccluster(wrap_do_analyze(i));
 INSERT INTO vaccluster VALUES (1), (2);
 ANALYZE vaccluster;