Prevent indirect security attacks via changing session-local state within

an allegedly immutable index function.  It was previously recognized that
we had to prevent such a function from executing SET/RESET ROLE/SESSION
AUTHORIZATION, or it could trivially obtain the privileges of the session
user.  However, since there is in general no privilege checking for changes
of session-local state, it is also possible for such a function to change
settings in a way that might subvert later operations in the same session.
Examples include changing search_path to cause an unexpected function to
be called, or replacing an existing prepared statement with another one
that will execute a function of the attacker's choosing.

The present patch secures VACUUM, ANALYZE, and CREATE INDEX/REINDEX against
these threats, which are the same places previously deemed to need protection
against the SET ROLE issue.  GUC changes are still allowed, since there are
many useful cases for that, but we prevent security problems by forcing a
rollback of any GUC change after completing the operation.  Other cases are
handled by throwing an error if any change is attempted; these include temp
table creation, closing a cursor, and creating or deleting a prepared
statement.  (In 7.4, the infrastructure to roll back GUC changes doesn't
exist, so we settle for rejecting changes of "search_path" in these contexts.)

Original report and patch by Gurjeet Singh, additional analysis by
Tom Lane.

Security: CVE-2009-4136

4 files changed, 51 insertions, 24 deletions

diff --git a/src/backend/commands/analyze.c b/src/backend/commands/analyze.c
index 8a76782f13a..720eb33f96f 100644
--- a/src/backend/commands/analyze.c
+++ b/src/backend/commands/analyze.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/commands/analyze.c,v 1.101.2.2 2009/05/19 08:30:18 heikki Exp $
+ *	  $PostgreSQL: pgsql/src/backend/commands/analyze.c,v 1.101.2.3 2009/12/09 21:58:28 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -31,6 +31,7 @@
 #include "pgstat.h"
 #include "utils/acl.h"
 #include "utils/datum.h"
+#include "utils/guc.h"
 #include "utils/lsyscache.h"
 #include "utils/memutils.h"
 #include "utils/syscache.h"
@@ -110,7 +111,8 @@ analyze_rel(Oid relid, VacuumStmt *vacstmt)
 				totaldeadrows;
 	HeapTuple  *rows;
 	Oid			save_userid;
-	bool		save_secdefcxt;
+	int			save_sec_context;
+	int			save_nestlevel;
 
 	if (vacstmt->verbose)
 		elevel = INFO;
@@ -198,11 +200,14 @@ analyze_rel(Oid relid, VacuumStmt *vacstmt)
 					RelationGetRelationName(onerel))));
 
 	/*
-	 * Switch to the table owner's userid, so that any index functions are
-	 * run as that user.
+	 * Switch to the table owner's userid, so that any index functions are run
+	 * as that user.  Also lock down security-restricted operations and
+	 * arrange to make GUC variable changes local to this command.
 	 */
-	GetUserIdAndContext(&save_userid, &save_secdefcxt);
-	SetUserIdAndContext(onerel->rd_rel->relowner, true);
+	GetUserIdAndSecContext(&save_userid, &save_sec_context);
+	SetUserIdAndSecContext(onerel->rd_rel->relowner,
+						   save_sec_context | SECURITY_RESTRICTED_OPERATION);
+	save_nestlevel = NewGUCNestLevel();
 
 	/*
 	 * Determine which columns to analyze
@@ -450,8 +455,11 @@ cleanup:
 	 */
 	relation_close(onerel, NoLock);
 
-	/* Restore userid */
-	SetUserIdAndContext(save_userid, save_secdefcxt);
+	/* Roll back any GUC changes executed by index functions */
+	AtEOXact_GUC(false, save_nestlevel);
+
+	/* Restore userid and security context */
+	SetUserIdAndSecContext(save_userid, save_sec_context);
 }
 
 /*
diff --git a/src/backend/commands/schemacmds.c b/src/backend/commands/schemacmds.c
index 881f45fa2e8..1da750561eb 100644
--- a/src/backend/commands/schemacmds.c
+++ b/src/backend/commands/schemacmds.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/commands/schemacmds.c,v 1.41.2.1 2008/01/03 21:23:45 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/commands/schemacmds.c,v 1.41.2.2 2009/12/09 21:58:28 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -47,10 +47,10 @@ CreateSchemaCommand(CreateSchemaStmt *stmt)
 	ListCell   *parsetree_item;
 	Oid			owner_uid;
 	Oid			saved_uid;
-	bool		saved_secdefcxt;
+	int			save_sec_context;
 	AclResult	aclresult;
 
-	GetUserIdAndContext(&saved_uid, &saved_secdefcxt);
+	GetUserIdAndSecContext(&saved_uid, &save_sec_context);
 
 	/*
 	 * Who is supposed to own the new schema?
@@ -90,7 +90,8 @@ CreateSchemaCommand(CreateSchemaStmt *stmt)
 	 * of error, transaction abort will clean things up.)
 	 */
 	if (saved_uid != owner_uid)
-		SetUserIdAndContext(owner_uid, true);
+		SetUserIdAndSecContext(owner_uid,
+							   save_sec_context | SECURITY_LOCAL_USERID_CHANGE);
 
 	/* Create the schema's namespace */
 	namespaceId = NamespaceCreate(schemaName, owner_uid);
@@ -141,8 +142,8 @@ CreateSchemaCommand(CreateSchemaStmt *stmt)
 	/* Reset search path to normal state */
 	PopSpecialNamespace(namespaceId);
 
-	/* Reset current user */
-	SetUserIdAndContext(saved_uid, saved_secdefcxt);
+	/* Reset current user and security context */
+	SetUserIdAndSecContext(saved_uid, save_sec_context);
 }
 
 
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 36f802eb1e6..8997fea5d99 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/commands/tablecmds.c,v 1.206.2.7 2008/10/07 11:15:54 heikki Exp $
+ *	  $PostgreSQL: pgsql/src/backend/commands/tablecmds.c,v 1.206.2.8 2009/12/09 21:58:28 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -302,6 +302,16 @@ DefineRelation(CreateStmt *stmt, char relkind)
 				 errmsg("ON COMMIT can only be used on temporary tables")));
 
 	/*
+	 * Security check: disallow creating temp tables from security-restricted
+	 * code.  This is needed because calling code might not expect untrusted
+	 * tables to appear in pg_temp at the front of its search path.
+	 */
+	if (stmt->relation->istemp && InSecurityRestrictedOperation())
+		ereport(ERROR,
+				(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+				 errmsg("cannot create temporary table within security-restricted operation")));
+
+	/*
 	 * Look up the namespace in which we are supposed to create the relation.
 	 * Check we have permission to create there. Skip check if bootstrapping,
 	 * since permissions machinery may not be working yet.
diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c
index ffa986c0538..ef80492911a 100644
--- a/src/backend/commands/vacuum.c
+++ b/src/backend/commands/vacuum.c
@@ -13,7 +13,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/commands/vacuum.c,v 1.342.2.7 2009/11/10 18:00:57 alvherre Exp $
+ *	  $PostgreSQL: pgsql/src/backend/commands/vacuum.c,v 1.342.2.8 2009/12/09 21:58:29 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -41,6 +41,7 @@
 #include "utils/builtins.h"
 #include "utils/flatfiles.h"
 #include "utils/fmgroids.h"
+#include "utils/guc.h"
 #include "utils/inval.h"
 #include "utils/lsyscache.h"
 #include "utils/memutils.h"
@@ -958,7 +959,8 @@ vacuum_rel(Oid relid, VacuumStmt *vacstmt, char expected_relkind)
 	LockRelId	onerelid;
 	Oid			toast_relid;
 	Oid			save_userid;
-	bool		save_secdefcxt;
+	int			save_sec_context;
+	int			save_nestlevel;
 	bool		heldoff;
 
 	/* Begin a transaction for vacuuming this relation */
@@ -1086,12 +1088,15 @@ vacuum_rel(Oid relid, VacuumStmt *vacstmt, char expected_relkind)
 	toast_relid = onerel->rd_rel->reltoastrelid;
 
 	/*
-	 * Switch to the table owner's userid, so that any index functions are
-	 * run as that user.  (This is unnecessary, but harmless, for lazy
-	 * VACUUM.)
+	 * Switch to the table owner's userid, so that any index functions are run
+	 * as that user.  Also lock down security-restricted operations and
+	 * arrange to make GUC variable changes local to this command.
+	 * (This is unnecessary, but harmless, for lazy VACUUM.)
 	 */
-	GetUserIdAndContext(&save_userid, &save_secdefcxt);
-	SetUserIdAndContext(onerel->rd_rel->relowner, true);
+	GetUserIdAndSecContext(&save_userid, &save_sec_context);
+	SetUserIdAndSecContext(onerel->rd_rel->relowner,
+						   save_sec_context | SECURITY_RESTRICTED_OPERATION);
+	save_nestlevel = NewGUCNestLevel();
 
 	/*
 	 * Tell the cache replacement strategy that vacuum is causing all
@@ -1109,8 +1114,11 @@ vacuum_rel(Oid relid, VacuumStmt *vacstmt, char expected_relkind)
 
 	StrategyHintVacuum(false);
 
-	/* Restore userid */
-	SetUserIdAndContext(save_userid, save_secdefcxt);
+	/* Roll back any GUC changes executed by index functions */
+	AtEOXact_GUC(false, save_nestlevel);
+
+	/* Restore userid and security context */
+	SetUserIdAndSecContext(save_userid, save_sec_context);
 
 	/* all done with this class, but hold lock until commit */
 	relation_close(onerel, NoLock);