0001-Restrict-search_path-for-non-owners-performing-maint.patch

From 67387349d2d84cdc2d7b6d7ab5d5824e5ccc89bf Mon Sep 17 00:00:00 2001
From: Jeff Davis <jeff@j-davis.com>
Date: Thu, 29 Jun 2023 16:03:53 -0700
Subject: [PATCH] Restrict search_path for non-owners performing maintenance.

When non-owners execute 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 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.

A previous fix 05e1737351 was deemed too large a change. This is a
narrower fix that only affects non-owners, which previously could not
execute maintenance commands. Per suggestion from David G. Johnston.

Discussion: https://postgr.es/m/CAKFQuwaVJkM9u%2BqpOaom2UkPE1sz0BASF-E5amxWPxncUhm4Hw%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          |  5 +++++
 src/backend/commands/analyze.c       |  2 ++
 src/backend/commands/cluster.c       |  3 ++-
 src/backend/commands/indexcmds.c     |  4 +++-
 src/backend/commands/matview.c       |  2 ++
 src/backend/commands/vacuum.c        |  2 ++
 src/backend/utils/init/usercontext.c | 17 +++++++++++++++++
 src/include/utils/guc.h              |  6 ++++++
 src/include/utils/usercontext.h      |  1 +
 11 files changed, 44 insertions(+), 2 deletions(-)

diff --git a/contrib/amcheck/verify_nbtree.c b/contrib/amcheck/verify_nbtree.c
index 94a9759322..1a77035e25 100644
--- a/contrib/amcheck/verify_nbtree.c
+++ b/contrib/amcheck/verify_nbtree.c
@@ -40,6 +40,7 @@
 #include "utils/guc.h"
 #include "utils/memutils.h"
 #include "utils/snapmgr.h"
+#include "utils/usercontext.h"
 
 
 PG_MODULE_MAGIC;
@@ -281,6 +282,7 @@ bt_index_check_internal(Oid indrelid, bool parentcheck, bool heapallindexed,
 		SetUserIdAndSecContext(heaprel->rd_rel->relowner,
 							   save_sec_context | SECURITY_RESTRICTED_OPERATION);
 		save_nestlevel = NewGUCNestLevel();
+		RestrictSearchPath(save_userid, heaprel->rd_rel->relowner);
 	}
 	else
 	{
diff --git a/src/backend/access/brin/brin.c b/src/backend/access/brin/brin.c
index 3c6a956eaa..9e04fd6ecb 100644
--- a/src/backend/access/brin/brin.c
+++ b/src/backend/access/brin/brin.c
@@ -40,6 +40,7 @@
 #include "utils/index_selfuncs.h"
 #include "utils/memutils.h"
 #include "utils/rel.h"
+#include "utils/usercontext.h"
 
 
 /*
@@ -1066,6 +1067,7 @@ brin_summarize_range(PG_FUNCTION_ARGS)
 		SetUserIdAndSecContext(heapRel->rd_rel->relowner,
 							   save_sec_context | SECURITY_RESTRICTED_OPERATION);
 		save_nestlevel = NewGUCNestLevel();
+		RestrictSearchPath(save_userid, heapRel->rd_rel->relowner);
 	}
 	else
 	{
diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
index 352e43d0e6..9123e7e7b7 100644
--- a/src/backend/catalog/index.c
+++ b/src/backend/catalog/index.c
@@ -84,6 +84,7 @@
 #include "utils/snapmgr.h"
 #include "utils/syscache.h"
 #include "utils/tuplesort.h"
+#include "utils/usercontext.h"
 
 /* Potentially set by pg_upgrade_support functions */
 Oid			binary_upgrade_next_index_pg_class_oid = InvalidOid;
@@ -1475,6 +1476,7 @@ index_concurrently_build(Oid heapRelationId,
 	SetUserIdAndSecContext(heapRel->rd_rel->relowner,
 						   save_sec_context | SECURITY_RESTRICTED_OPERATION);
 	save_nestlevel = NewGUCNestLevel();
+	RestrictSearchPath(save_userid, heapRel->rd_rel->relowner);
 
 	indexRelation = index_open(indexRelationId, RowExclusiveLock);
 
@@ -3006,6 +3008,7 @@ index_build(Relation heapRelation,
 	SetUserIdAndSecContext(heapRelation->rd_rel->relowner,
 						   save_sec_context | SECURITY_RESTRICTED_OPERATION);
 	save_nestlevel = NewGUCNestLevel();
+	RestrictSearchPath(save_userid, heapRelation->rd_rel->relowner);
 
 	/* Set up initial progress report status */
 	{
@@ -3341,6 +3344,7 @@ validate_index(Oid heapId, Oid indexId, Snapshot snapshot)
 	SetUserIdAndSecContext(heapRelation->rd_rel->relowner,
 						   save_sec_context | SECURITY_RESTRICTED_OPERATION);
 	save_nestlevel = NewGUCNestLevel();
+	RestrictSearchPath(save_userid, heapRelation->rd_rel->relowner);
 
 	indexRelation = index_open(indexId, RowExclusiveLock);
 
@@ -3601,6 +3605,7 @@ reindex_index(Oid indexId, bool skip_constraint_checks, char persistence,
 	SetUserIdAndSecContext(heapRelation->rd_rel->relowner,
 						   save_sec_context | SECURITY_RESTRICTED_OPERATION);
 	save_nestlevel = NewGUCNestLevel();
+	RestrictSearchPath(save_userid, heapRelation->rd_rel->relowner);
 
 	if (progress)
 	{
diff --git a/src/backend/commands/analyze.c b/src/backend/commands/analyze.c
index fc9a371f9b..cbe8350a7e 100644
--- a/src/backend/commands/analyze.c
+++ b/src/backend/commands/analyze.c
@@ -67,6 +67,7 @@
 #include "utils/spccache.h"
 #include "utils/syscache.h"
 #include "utils/timestamp.h"
+#include "utils/usercontext.h"
 
 
 /* Per-index data for ANALYZE */
@@ -348,6 +349,7 @@ do_analyze_rel(Relation onerel, VacuumParams *params,
 	SetUserIdAndSecContext(onerel->rd_rel->relowner,
 						   save_sec_context | SECURITY_RESTRICTED_OPERATION);
 	save_nestlevel = NewGUCNestLevel();
+	RestrictSearchPath(save_userid, onerel->rd_rel->relowner);
 
 	/* 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..efed6401ae 100644
--- a/src/backend/commands/cluster.c
+++ b/src/backend/commands/cluster.c
@@ -59,6 +59,7 @@
 #include "utils/snapmgr.h"
 #include "utils/syscache.h"
 #include "utils/tuplesort.h"
+#include "utils/usercontext.h"
 
 /*
  * This struct is used to pass around the information on tables to be
@@ -355,7 +356,7 @@ cluster_rel(Oid tableOid, Oid indexOid, ClusterParams *params)
 	SetUserIdAndSecContext(OldHeap->rd_rel->relowner,
 						   save_sec_context | SECURITY_RESTRICTED_OPERATION);
 	save_nestlevel = NewGUCNestLevel();
-
+	RestrictSearchPath(save_userid, OldHeap->rd_rel->relowner);
 	/*
 	 * Since we may open a new transaction for each relation, we have to check
 	 * that the relation still is what we think it is.
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index 9bc97e1fc2..f4dead0f48 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -69,7 +69,7 @@
 #include "utils/regproc.h"
 #include "utils/snapmgr.h"
 #include "utils/syscache.h"
-
+#include "utils/usercontext.h"
 
 /* non-export function prototypes */
 static bool CompareOpclassOptions(Datum *opts1, Datum *opts2, int natts);
@@ -1300,6 +1300,7 @@ DefineIndex(Oid relationId,
 				SetUserIdAndSecContext(childrel->rd_rel->relowner,
 									   child_save_sec_context | SECURITY_RESTRICTED_OPERATION);
 				child_save_nestlevel = NewGUCNestLevel();
+				RestrictSearchPath(child_save_userid, childrel->rd_rel->relowner);
 
 				/*
 				 * Don't try to create indexes on foreign tables, though. Skip
@@ -3750,6 +3751,7 @@ ReindexRelationConcurrently(Oid relationOid, ReindexParams *params)
 		SetUserIdAndSecContext(heapRel->rd_rel->relowner,
 							   save_sec_context | SECURITY_RESTRICTED_OPERATION);
 		save_nestlevel = NewGUCNestLevel();
+		RestrictSearchPath(save_userid, heapRel->rd_rel->relowner);
 
 		/* 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..920703442b 100644
--- a/src/backend/commands/matview.c
+++ b/src/backend/commands/matview.c
@@ -45,6 +45,7 @@
 #include "utils/rel.h"
 #include "utils/snapmgr.h"
 #include "utils/syscache.h"
+#include "utils/usercontext.h"
 
 
 typedef struct
@@ -179,6 +180,7 @@ ExecRefreshMatView(RefreshMatViewStmt *stmt, const char *queryString,
 	SetUserIdAndSecContext(relowner,
 						   save_sec_context | SECURITY_RESTRICTED_OPERATION);
 	save_nestlevel = NewGUCNestLevel();
+	RestrictSearchPath(save_userid, relowner);
 
 	/* 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..58caa761ac 100644
--- a/src/backend/commands/vacuum.c
+++ b/src/backend/commands/vacuum.c
@@ -61,6 +61,7 @@
 #include "utils/pg_rusage.h"
 #include "utils/snapmgr.h"
 #include "utils/syscache.h"
+#include "utils/usercontext.h"
 
 
 /*
@@ -2171,6 +2172,7 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params,
 	SetUserIdAndSecContext(rel->rd_rel->relowner,
 						   save_sec_context | SECURITY_RESTRICTED_OPERATION);
 	save_nestlevel = NewGUCNestLevel();
+	RestrictSearchPath(save_userid, rel->rd_rel->relowner);
 
 	/*
 	 * If PROCESS_MAIN is set (the default), it's time to vacuum the main
diff --git a/src/backend/utils/init/usercontext.c b/src/backend/utils/init/usercontext.c
index dd9a0dd6a8..b62420b56e 100644
--- a/src/backend/utils/init/usercontext.c
+++ b/src/backend/utils/init/usercontext.c
@@ -90,3 +90,20 @@ RestoreUserContext(UserContext *context)
 		AtEOXact_GUC(false, context->save_nestlevel);
 	SetUserIdAndSecContext(context->save_userid, context->save_sec_context);
 }
+
+/*
+ * When potentially executing code on an object with the given owner, if
+ * userid does not have owner privileges, set a restricted safe search_path.
+ *
+ * Note: the caller is expected to save and restore the GUC nest level as
+ * necessary.
+ */
+void
+RestrictSearchPath(Oid userid, Oid owner)
+{
+	if (has_privs_of_role(userid, owner))
+		return;
+
+	SetConfigOption("search_path", GUC_SAFE_SEARCH_PATH, PGC_USERSET,
+					PGC_S_SESSION);
+}
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/include/utils/usercontext.h b/src/include/utils/usercontext.h
index a8195c194d..d5422ce70d 100644
--- a/src/include/utils/usercontext.h
+++ b/src/include/utils/usercontext.h
@@ -22,5 +22,6 @@ typedef struct UserContext
 /* Function prototypes. */
 extern void SwitchToUntrustedUser(Oid userid, UserContext *context);
 extern void RestoreUserContext(UserContext *context);
+extern void RestrictSearchPath(Oid userid, Oid owner);
 
 #endif							/* USERCONTEXT_H */
-- 
2.34.1