Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 05e1737

Browse files
jeff-davisjeff-davis
committed
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 60684dd, 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
1 parent 9aee26a commit 05e1737

File tree

15 files changed

+48
-16
lines changed

15 files changed

+48
-16
lines changed

‎contrib/amcheck/verify_nbtree.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -282,6 +282,8 @@ bt_index_check_internal(Oid indrelid, bool parentcheck, bool heapallindexed,
282282
SetUserIdAndSecContext(heaprel->rd_rel->relowner,
283283
save_sec_context | SECURITY_RESTRICTED_OPERATION);
284284
save_nestlevel = NewGUCNestLevel();
285+
SetConfigOption("search_path", GUC_SAFE_SEARCH_PATH, PGC_USERSET,
286+
PGC_S_SESSION);
285287
}
286288
else
287289
{

‎src/backend/access/brin/brin.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1066,6 +1066,8 @@ brin_summarize_range(PG_FUNCTION_ARGS)
10661066
SetUserIdAndSecContext(heapRel->rd_rel->relowner,
10671067
save_sec_context | SECURITY_RESTRICTED_OPERATION);
10681068
save_nestlevel = NewGUCNestLevel();
1069+
SetConfigOption("search_path", GUC_SAFE_SEARCH_PATH, PGC_USERSET,
1070+
PGC_S_SESSION);
10691071
}
10701072
else
10711073
{

‎src/backend/catalog/index.c

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1475,6 +1475,8 @@ index_concurrently_build(Oid heapRelationId,
14751475
SetUserIdAndSecContext(heapRel->rd_rel->relowner,
14761476
save_sec_context | SECURITY_RESTRICTED_OPERATION);
14771477
save_nestlevel = NewGUCNestLevel();
1478+
SetConfigOption("search_path", GUC_SAFE_SEARCH_PATH, PGC_USERSET,
1479+
PGC_S_SESSION);
14781480

14791481
indexRelation = index_open(indexRelationId, RowExclusiveLock);
14801482

@@ -3006,6 +3008,8 @@ index_build(Relation heapRelation,
30063008
SetUserIdAndSecContext(heapRelation->rd_rel->relowner,
30073009
save_sec_context | SECURITY_RESTRICTED_OPERATION);
30083010
save_nestlevel = NewGUCNestLevel();
3011+
SetConfigOption("search_path", GUC_SAFE_SEARCH_PATH, PGC_USERSET,
3012+
PGC_S_SESSION);
30093013

30103014
/* Set up initial progress report status */
30113015
{
@@ -3341,6 +3345,8 @@ validate_index(Oid heapId, Oid indexId, Snapshot snapshot)
33413345
SetUserIdAndSecContext(heapRelation->rd_rel->relowner,
33423346
save_sec_context | SECURITY_RESTRICTED_OPERATION);
33433347
save_nestlevel = NewGUCNestLevel();
3348+
SetConfigOption("search_path", GUC_SAFE_SEARCH_PATH, PGC_USERSET,
3349+
PGC_S_SESSION);
33443350

33453351
indexRelation = index_open(indexId, RowExclusiveLock);
33463352

@@ -3601,6 +3607,8 @@ reindex_index(Oid indexId, bool skip_constraint_checks, char persistence,
36013607
SetUserIdAndSecContext(heapRelation->rd_rel->relowner,
36023608
save_sec_context | SECURITY_RESTRICTED_OPERATION);
36033609
save_nestlevel = NewGUCNestLevel();
3610+
SetConfigOption("search_path", GUC_SAFE_SEARCH_PATH, PGC_USERSET,
3611+
PGC_S_SESSION);
36043612

36053613
if (progress)
36063614
{

‎src/backend/commands/analyze.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -348,6 +348,8 @@ do_analyze_rel(Relation onerel, VacuumParams *params,
348348
SetUserIdAndSecContext(onerel->rd_rel->relowner,
349349
save_sec_context | SECURITY_RESTRICTED_OPERATION);
350350
save_nestlevel = NewGUCNestLevel();
351+
SetConfigOption("search_path", GUC_SAFE_SEARCH_PATH, PGC_USERSET,
352+
PGC_S_SESSION);
351353

352354
/* measure elapsed time iff autovacuum logging requires it */
353355
if (IsAutoVacuumWorkerProcess() && params->log_min_duration >= 0)

‎src/backend/commands/cluster.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -355,6 +355,8 @@ cluster_rel(Oid tableOid, Oid indexOid, ClusterParams *params)
355355
SetUserIdAndSecContext(OldHeap->rd_rel->relowner,
356356
save_sec_context | SECURITY_RESTRICTED_OPERATION);
357357
save_nestlevel = NewGUCNestLevel();
358+
SetConfigOption("search_path", GUC_SAFE_SEARCH_PATH, PGC_USERSET,
359+
PGC_S_SESSION);
358360

359361
/*
360362
* Since we may open a new transaction for each relation, we have to check

‎src/backend/commands/indexcmds.c

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -575,6 +575,8 @@ DefineIndex(Oid relationId,
575575
int root_save_nestlevel;
576576

577577
root_save_nestlevel = NewGUCNestLevel();
578+
SetConfigOption("search_path", GUC_SAFE_SEARCH_PATH, PGC_USERSET,
579+
PGC_S_SESSION);
578580

579581
/*
580582
* Some callers need us to run with an empty default_tablespace; this is a
@@ -1300,6 +1302,8 @@ DefineIndex(Oid relationId,
13001302
SetUserIdAndSecContext(childrel->rd_rel->relowner,
13011303
child_save_sec_context | SECURITY_RESTRICTED_OPERATION);
13021304
child_save_nestlevel = NewGUCNestLevel();
1305+
SetConfigOption("search_path", GUC_SAFE_SEARCH_PATH, PGC_USERSET,
1306+
PGC_S_SESSION);
13031307

13041308
/*
13051309
* Don't try to create indexes on foreign tables, though. Skip
@@ -3753,6 +3757,8 @@ ReindexRelationConcurrently(Oid relationOid, ReindexParams *params)
37533757
SetUserIdAndSecContext(heapRel->rd_rel->relowner,
37543758
save_sec_context | SECURITY_RESTRICTED_OPERATION);
37553759
save_nestlevel = NewGUCNestLevel();
3760+
SetConfigOption("search_path", GUC_SAFE_SEARCH_PATH, PGC_USERSET,
3761+
PGC_S_SESSION);
37563762

37573763
/* determine safety of this index for set_indexsafe_procflags */
37583764
idx->safe = (indexRel->rd_indexprs == NIL &&

‎src/backend/commands/matview.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -179,6 +179,8 @@ ExecRefreshMatView(RefreshMatViewStmt *stmt, const char *queryString,
179179
SetUserIdAndSecContext(relowner,
180180
save_sec_context | SECURITY_RESTRICTED_OPERATION);
181181
save_nestlevel = NewGUCNestLevel();
182+
SetConfigOption("search_path", GUC_SAFE_SEARCH_PATH, PGC_USERSET,
183+
PGC_S_SESSION);
182184

183185
/* Make sure it is a materialized view. */
184186
if (matviewRel->rd_rel->relkind != RELKIND_MATVIEW)

‎src/backend/commands/vacuum.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2172,6 +2172,8 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params,
21722172
SetUserIdAndSecContext(rel->rd_rel->relowner,
21732173
save_sec_context | SECURITY_RESTRICTED_OPERATION);
21742174
save_nestlevel = NewGUCNestLevel();
2175+
SetConfigOption("search_path", GUC_SAFE_SEARCH_PATH, PGC_USERSET,
2176+
PGC_S_SESSION);
21752177

21762178
/*
21772179
* If PROCESS_MAIN is set (the default), it's time to vacuum the main

‎src/bin/scripts/t/100_vacuumdb.pl

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -109,15 +109,11 @@
109109
CREATE FUNCTION f1(int) RETURNS int LANGUAGE SQL AS 'SELECT f0($1)';
110110
CREATE TABLE funcidx (x int);
111111
INSERT INTO funcidx VALUES (0),(1),(2),(3);
112-
CREATE INDEX i0 ON funcidx ((f1(x)));
113112
CREATE SCHEMA "Foo";
114113
CREATE TABLE "Foo".bar(id int);
115114
|);
116115
$node->command_ok([qw|vacuumdb -Z --table="need""q(uot"(")x") postgres|],
117116
'column list');
118-
$node->command_fails(
119-
[qw|vacuumdb -Zt funcidx postgres|],
120-
'unqualified name via functional index');
121117

122118
$node->command_fails(
123119
[ 'vacuumdb', '--analyze', '--table', 'vactable(c)', 'postgres' ],

‎src/include/utils/guc.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -201,6 +201,12 @@ typedef enum
201201

202202
#define GUC_QUALIFIER_SEPARATOR '.'
203203

204+
/*
205+
* Safe search path when executing code as the table owner, such as during
206+
* maintenance operations.
207+
*/
208+
#define GUC_SAFE_SEARCH_PATH "pg_catalog, pg_temp"
209+
204210
/*
205211
* Bit values in "flags" of a GUC variable. Note that these don't appear
206212
* on disk, so we can reassign their values freely.

‎src/test/modules/test_oat_hooks/expected/test_oat_hooks.out

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,11 +89,15 @@ NOTICE: in object access: superuser finished create (subId=0x0) [internal]
8989
NOTICE: in process utility: superuser finished CREATE TABLE
9090
CREATE INDEX regress_test_table_t_idx ON regress_test_table (t);
9191
NOTICE: in process utility: superuser attempting CREATE INDEX
92+
NOTICE: in object access: superuser attempting namespace search (subId=0x0) [no report on violation, allowed]
93+
NOTICE: in object access: superuser finished namespace search (subId=0x0) [no report on violation, allowed]
9294
NOTICE: in object access: superuser attempting create (subId=0x0) [explicit]
9395
NOTICE: in object access: superuser finished create (subId=0x0) [explicit]
9496
NOTICE: in process utility: superuser finished CREATE INDEX
9597
GRANT SELECT ON Table regress_test_table TO public;
9698
NOTICE: in process utility: superuser attempting GRANT
99+
NOTICE: in object access: superuser attempting namespace search (subId=0x0) [no report on violation, allowed]
100+
NOTICE: in object access: superuser finished namespace search (subId=0x0) [no report on violation, allowed]
97101
NOTICE: in process utility: superuser finished GRANT
98102
CREATE FUNCTION regress_test_func (t text) RETURNS text AS $$
99103
SELECT $1;

‎src/test/regress/expected/privileges.out

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1769,7 +1769,7 @@ SET SESSION AUTHORIZATION regress_sro_user;
17691769
CREATE FUNCTION unwanted_grant() RETURNS void LANGUAGE sql AS
17701770
'GRANT regress_priv_group2 TO regress_sro_user';
17711771
CREATE FUNCTION mv_action() RETURNS bool LANGUAGE sql AS
1772-
'DECLARE c CURSOR WITH HOLD FOR SELECT unwanted_grant(); SELECT true';
1772+
'DECLARE c CURSOR WITH HOLD FOR SELECT public.unwanted_grant(); SELECT true';
17731773
-- REFRESH of this MV will queue a GRANT at end of transaction
17741774
CREATE MATERIALIZED VIEW sro_mv AS SELECT mv_action() WITH NO DATA;
17751775
REFRESH MATERIALIZED VIEW sro_mv;
@@ -1783,12 +1783,12 @@ SET SESSION AUTHORIZATION regress_sro_user;
17831783
-- INSERT to this table will queue a GRANT at end of transaction
17841784
CREATE TABLE sro_trojan_table ();
17851785
CREATE FUNCTION sro_trojan() RETURNS trigger LANGUAGE plpgsql AS
1786-
'BEGIN PERFORM unwanted_grant(); RETURN NULL; END';
1786+
'BEGIN PERFORM public.unwanted_grant(); RETURN NULL; END';
17871787
CREATE CONSTRAINT TRIGGER t AFTER INSERT ON sro_trojan_table
17881788
INITIALLY DEFERRED FOR EACH ROW EXECUTE PROCEDURE sro_trojan();
17891789
-- Now, REFRESH will issue such an INSERT, queueing the GRANT
17901790
CREATE OR REPLACE FUNCTION mv_action() RETURNS bool LANGUAGE sql AS
1791-
'INSERT INTO sro_trojan_table DEFAULT VALUES; SELECT true';
1791+
'INSERT INTO public.sro_trojan_table DEFAULT VALUES; SELECT true';
17921792
REFRESH MATERIALIZED VIEW sro_mv;
17931793
ERROR: cannot fire deferred trigger within security-restricted operation
17941794
CONTEXT: SQL function "mv_action" statement 1
@@ -1800,15 +1800,15 @@ BEGIN; SET CONSTRAINTS ALL IMMEDIATE; REFRESH MATERIALIZED VIEW sro_mv; COMMIT;
18001800
ERROR: permission denied to grant role "regress_priv_group2"
18011801
DETAIL: Only roles with the ADMIN option on role "regress_priv_group2" may grant this role.
18021802
CONTEXT: SQL function "unwanted_grant" statement 1
1803-
SQL statement "SELECT unwanted_grant()"
1804-
PL/pgSQL function sro_trojan() line 1 at PERFORM
1803+
SQL statement "SELECT public.unwanted_grant()"
1804+
PL/pgSQL function public.sro_trojan() line 1 at PERFORM
18051805
SQL function "mv_action" statement 1
18061806
-- REFRESH MATERIALIZED VIEW CONCURRENTLY use of eval_const_expressions()
18071807
SET SESSION AUTHORIZATION regress_sro_user;
18081808
CREATE FUNCTION unwanted_grant_nofail(int) RETURNS int
18091809
IMMUTABLE LANGUAGE plpgsql AS $$
18101810
BEGIN
1811-
PERFORM unwanted_grant();
1811+
PERFORM public.unwanted_grant();
18121812
RAISE WARNING 'owned';
18131813
RETURN 1;
18141814
EXCEPTION WHEN OTHERS THEN

‎src/test/regress/expected/vacuum.out

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ CLUSTER vaccluster;
6464
CREATE FUNCTION do_analyze() RETURNS VOID VOLATILE LANGUAGE SQL
6565
AS 'ANALYZE pg_am';
6666
CREATE FUNCTION wrap_do_analyze(c INT) RETURNS INT IMMUTABLE LANGUAGE SQL
67-
AS 'SELECT $1 FROM do_analyze()';
67+
AS 'SELECT $1 FROM public.do_analyze()';
6868
CREATE INDEX ON vaccluster(wrap_do_analyze(i));
6969
INSERT INTO vaccluster VALUES (1), (2);
7070
ANALYZE vaccluster;

‎src/test/regress/sql/privileges.sql

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1177,7 +1177,7 @@ SET SESSION AUTHORIZATION regress_sro_user;
11771177
CREATE FUNCTION unwanted_grant() RETURNS void LANGUAGE sql AS
11781178
'GRANT regress_priv_group2 TO regress_sro_user';
11791179
CREATE FUNCTION mv_action() RETURNS bool LANGUAGE sql AS
1180-
'DECLARE c CURSOR WITH HOLD FOR SELECT unwanted_grant(); SELECT true';
1180+
'DECLARE c CURSOR WITH HOLD FOR SELECT public.unwanted_grant(); SELECT true';
11811181
-- REFRESH of this MV will queue a GRANT at end of transaction
11821182
CREATE MATERIALIZED VIEW sro_mv AS SELECT mv_action() WITH NO DATA;
11831183
REFRESH MATERIALIZED VIEW sro_mv;
@@ -1188,12 +1188,12 @@ SET SESSION AUTHORIZATION regress_sro_user;
11881188
-- INSERT to this table will queue a GRANT at end of transaction
11891189
CREATE TABLE sro_trojan_table ();
11901190
CREATE FUNCTION sro_trojan() RETURNS trigger LANGUAGE plpgsql AS
1191-
'BEGIN PERFORM unwanted_grant(); RETURN NULL; END';
1191+
'BEGIN PERFORM public.unwanted_grant(); RETURN NULL; END';
11921192
CREATE CONSTRAINT TRIGGER t AFTER INSERT ON sro_trojan_table
11931193
INITIALLY DEFERRED FOR EACH ROW EXECUTE PROCEDURE sro_trojan();
11941194
-- Now, REFRESH will issue such an INSERT, queueing the GRANT
11951195
CREATE OR REPLACE FUNCTION mv_action() RETURNS bool LANGUAGE sql AS
1196-
'INSERT INTO sro_trojan_table DEFAULT VALUES; SELECT true';
1196+
'INSERT INTO public.sro_trojan_table DEFAULT VALUES; SELECT true';
11971197
REFRESH MATERIALIZED VIEW sro_mv;
11981198
\c -
11991199
REFRESH MATERIALIZED VIEW sro_mv;
@@ -1204,7 +1204,7 @@ SET SESSION AUTHORIZATION regress_sro_user;
12041204
CREATE FUNCTION unwanted_grant_nofail(int) RETURNS int
12051205
IMMUTABLE LANGUAGE plpgsql AS $$
12061206
BEGIN
1207-
PERFORM unwanted_grant();
1207+
PERFORM public.unwanted_grant();
12081208
RAISE WARNING 'owned';
12091209
RETURN 1;
12101210
EXCEPTION WHEN OTHERS THEN

‎src/test/regress/sql/vacuum.sql

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ CLUSTER vaccluster;
4949
CREATE FUNCTION do_analyze() RETURNS VOID VOLATILE LANGUAGE SQL
5050
AS 'ANALYZE pg_am';
5151
CREATE FUNCTION wrap_do_analyze(c INT) RETURNS INT IMMUTABLE LANGUAGE SQL
52-
AS 'SELECT $1 FROM do_analyze()';
52+
AS 'SELECT $1 FROM public.do_analyze()';
5353
CREATE INDEX ON vaccluster(wrap_do_analyze(i));
5454
INSERT INTO vaccluster VALUES (1), (2);
5555
ANALYZE vaccluster;

0 commit comments

Comments
 (0)