From 46dbbf86e927e1872004c37f651e6a5b5c62bdd0 Mon Sep 17 00:00:00 2001 From: Jeff Davis Date: Thu, 6 Jul 2023 13:06:22 -0700 Subject: [PATCH v5] Restrict search_path when performing maintenance. When executing maintenance operations (ANALYZE, CLUSTER, REFRESH MATERIALIZED VIEW, REINDEX, or VACUUM), set search_path to 'pg_catalog, pg_temp'. Functions that are used for functional indexes, in index expressions, or in materialized views and depend on a different search path should be declared with CREATE FUNCTION ... SET search_path='...'. This change was previously committed as 05e1737351, but was not acceptable for 16 and reverted. Discussion: https://postgr.es/m/CA%2BTgmoZVCHERUkXhAMT2Er-sKBc5C6_iX%2BTpxxivBevDHzq2TQ%40mail.gmail.com Discussion: https://postgr.es/m/e44327179e5c9015c8dda67351c04da552066017.camel%40j-davis.com --- contrib/amcheck/verify_nbtree.c | 2 ++ src/backend/access/brin/brin.c | 2 ++ src/backend/catalog/index.c | 8 ++++++++ src/backend/commands/analyze.c | 2 ++ src/backend/commands/cluster.c | 2 ++ src/backend/commands/indexcmds.c | 6 ++++++ src/backend/commands/matview.c | 2 ++ src/backend/commands/vacuum.c | 2 ++ src/bin/scripts/t/100_vacuumdb.pl | 4 ---- src/include/utils/guc.h | 6 ++++++ .../test_oat_hooks/expected/test_oat_hooks.out | 4 ++++ src/test/regress/expected/privileges.out | 12 ++++++------ src/test/regress/expected/vacuum.out | 2 +- src/test/regress/sql/privileges.sql | 8 ++++---- src/test/regress/sql/vacuum.sql | 2 +- 15 files changed, 48 insertions(+), 16 deletions(-) diff --git a/contrib/amcheck/verify_nbtree.c b/contrib/amcheck/verify_nbtree.c index 94a9759322..35035967f9 100644 --- a/contrib/amcheck/verify_nbtree.c +++ b/contrib/amcheck/verify_nbtree.c @@ -281,6 +281,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 3c6a956eaa..11e78cb62c 100644 --- 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 67b743e251..64127a894f 100644 --- a/src/backend/catalog/index.c +++ b/src/backend/catalog/index.c @@ -1476,6 +1476,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); @@ -3007,6 +3009,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 */ { @@ -3343,6 +3347,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); @@ -3603,6 +3609,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 9c8413eef2..c71051eaf2 100644 --- 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 03b24ab90f..393c732315 100644 --- 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 403f5fc143..8f182c1493 100644 --- 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 @@ -3767,6 +3771,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 f9a3bdfc3a..9114a7924c 100644 --- 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 7fe6a54c06..6dd0f1e77a 100644 --- a/src/backend/commands/vacuum.c +++ b/src/backend/commands/vacuum.c @@ -2171,6 +2171,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 eccfcc54a1..f91b5127a8 100644 --- 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 d5253c7ed2..edd82a551b 100644 --- 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 f80373aecc..effdc49145 100644 --- 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 3e4dfcc2ec..6457da9531 100644 --- 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 4def90b805..330fcd884c 100644 --- 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 134809e8cc..e961748d79 100644 --- 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 51d7b1fecc..0b63ef8dc6 100644 --- 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; -- 2.34.1