Make standard maintenance operations (including VACUUM, ANALYZE, REINDEX,

and CLUSTER) execute as the table owner rather than the calling user, using
the same privilege-switching mechanism already used for SECURITY DEFINER
functions.  The purpose of this change is to ensure that user-defined
functions used in index definitions cannot acquire the privileges of a
superuser account that is performing routine maintenance.  While a function
used in an index is supposed to be IMMUTABLE and thus not able to do anything
very interesting, there are several easy ways around that restriction; and
even if we could plug them all, there would remain a risk of reading sensitive
information and broadcasting it through a covert channel such as CPU usage.

To prevent bypassing this security measure, execution of SET SESSION
AUTHORIZATION and SET ROLE is now forbidden within a SECURITY DEFINER context.

Thanks to Itagaki Takahiro for reporting this vulnerability.

Security: CVE-2007-6600

src/backend/utils/adt/ri_triggers.c

@@ -15,7 +15,7 @@
  *
  * Portions Copyright (c) 1996-2008, PostgreSQL Global Development Group
  *
- * $PostgreSQL: pgsql/src/backend/utils/adt/ri_triggers.c,v 1.100 2008/01/01 19:45:52 momjian Exp $
+ * $PostgreSQL: pgsql/src/backend/utils/adt/ri_triggers.c,v 1.101 2008/01/03 21:23:15 tgl Exp $
  *
  * ----------
  */
@@ -3173,7 +3173,8 @@ ri_PlanCheck(const char *querystr, int nargs, Oid *argtypes,
 {
 	SPIPlanPtr	qplan;
 	Relation	query_rel;
-	Oid			save_uid;
+	Oid			save_userid;
+	bool		save_secdefcxt;
 
 	/*
 	 * The query is always run against the FK table except when this is an
@@ -3187,8 +3188,8 @@ ri_PlanCheck(const char *querystr, int nargs, Oid *argtypes,
 		query_rel = fk_rel;
 
 	/* Switch to proper UID to perform check as */
-	save_uid = GetUserId();
-	SetUserId(RelationGetForm(query_rel)->relowner);
+	GetUserIdAndContext(&save_userid, &save_secdefcxt);
+	SetUserIdAndContext(RelationGetForm(query_rel)->relowner, true);
 
 	/* Create the plan */
 	qplan = SPI_prepare(querystr, nargs, argtypes);
@@ -3197,7 +3198,7 @@ ri_PlanCheck(const char *querystr, int nargs, Oid *argtypes,
 		elog(ERROR, "SPI_prepare returned %d for %s", SPI_result, querystr);
 
 	/* Restore UID */
-	SetUserId(save_uid);
+	SetUserIdAndContext(save_userid, save_secdefcxt);
 
 	/* Save the plan if requested */
 	if (cache_plan)
@@ -3226,7 +3227,8 @@ ri_PerformCheck(RI_QueryKey *qkey, SPIPlanPtr qplan,
 	Snapshot	crosscheck_snapshot;
 	int			limit;
 	int			spi_result;
-	Oid			save_uid;
+	Oid			save_userid;
+	bool		save_secdefcxt;
 	Datum		vals[RI_MAX_NUMKEYS * 2];
 	char		nulls[RI_MAX_NUMKEYS * 2];
 
@@ -3304,8 +3306,8 @@ ri_PerformCheck(RI_QueryKey *qkey, SPIPlanPtr qplan,
 	limit = (expect_OK == SPI_OK_SELECT) ? 1 : 0;
 
 	/* Switch to proper UID to perform check as */
-	save_uid = GetUserId();
-	SetUserId(RelationGetForm(query_rel)->relowner);
+	GetUserIdAndContext(&save_userid, &save_secdefcxt);
+	SetUserIdAndContext(RelationGetForm(query_rel)->relowner, true);
 
 	/* Finally we can run the query. */
 	spi_result = SPI_execute_snapshot(qplan,
@@ -3314,7 +3316,7 @@ ri_PerformCheck(RI_QueryKey *qkey, SPIPlanPtr qplan,
 									  false, false, limit);
 
 	/* Restore UID */
-	SetUserId(save_uid);
+	SetUserIdAndContext(save_userid, save_secdefcxt);
 
 	/* Check result */
 	if (spi_result < 0)

src/backend/utils/fmgr/fmgr.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/utils/fmgr/fmgr.c,v 1.112 2008/01/01 19:45:53 momjian Exp $
+ *	  $PostgreSQL: pgsql/src/backend/utils/fmgr/fmgr.c,v 1.113 2008/01/03 21:23:15 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -864,6 +864,7 @@ fmgr_security_definer(PG_FUNCTION_ARGS)
 	struct fmgr_security_definer_cache *volatile fcache;
 	FmgrInfo   *save_flinfo;
 	Oid			save_userid;
+	bool		save_secdefcxt;
 	volatile int save_nestlevel;
 
 	if (!fcinfo->flinfo->fn_extra)
@@ -908,46 +909,50 @@ fmgr_security_definer(PG_FUNCTION_ARGS)
 	else
 		fcache = fcinfo->flinfo->fn_extra;
 
-	save_flinfo = fcinfo->flinfo;
-	/* GetUserId is cheap enough that no harm in a wasted call */
-	save_userid = GetUserId();
+	/* GetUserIdAndContext is cheap enough that no harm in a wasted call */
+	GetUserIdAndContext(&save_userid, &save_secdefcxt);
 	if (fcache->proconfig)		/* Need a new GUC nesting level */
 		save_nestlevel = NewGUCNestLevel();
 	else
 		save_nestlevel = 0;		/* keep compiler quiet */
 
+	if (OidIsValid(fcache->userid))
+		SetUserIdAndContext(fcache->userid, true);
+
+	if (fcache->proconfig)
+	{
+		ProcessGUCArray(fcache->proconfig,
+						(superuser() ? PGC_SUSET : PGC_USERSET),
+						PGC_S_SESSION,
+						GUC_ACTION_SAVE);
+	}
+
+	/*
+	 * We don't need to restore GUC or userid settings on error, because the
+	 * ensuing xact or subxact abort will do that.  The PG_TRY block is only
+	 * needed to clean up the flinfo link.
+	 */
+	save_flinfo = fcinfo->flinfo;
+
 	PG_TRY();
 	{
 		fcinfo->flinfo = &fcache->flinfo;
 
-		if (OidIsValid(fcache->userid))
-			SetUserId(fcache->userid);
-
-		if (fcache->proconfig)
-		{
-			ProcessGUCArray(fcache->proconfig,
-							(superuser() ? PGC_SUSET : PGC_USERSET),
-							PGC_S_SESSION,
-							GUC_ACTION_SAVE);
-		}
-
 		result = FunctionCallInvoke(fcinfo);
 	}
 	PG_CATCH();
 	{
 		fcinfo->flinfo = save_flinfo;
-		/* We don't need to restore GUC settings, outer xact abort will */
-		if (OidIsValid(fcache->userid))
-			SetUserId(save_userid);
 		PG_RE_THROW();
 	}
 	PG_END_TRY();
 
 	fcinfo->flinfo = save_flinfo;
+
 	if (fcache->proconfig)
 		AtEOXact_GUC(true, save_nestlevel);
 	if (OidIsValid(fcache->userid))
-		SetUserId(save_userid);
+		SetUserIdAndContext(save_userid, save_secdefcxt);
 
 	return result;
 }

src/backend/utils/init/miscinit.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/utils/init/miscinit.c,v 1.165 2008/01/01 19:45:53 momjian Exp $
+ *	  $PostgreSQL: pgsql/src/backend/utils/init/miscinit.c,v 1.166 2008/01/03 21:23:15 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -264,13 +264,15 @@ make_absolute_path(const char *path)
  * OuterUserId is the current user ID in effect at the "outer level" (outside
  * any transaction or function).  This is initially the same as SessionUserId,
  * but can be changed by SET ROLE to any role that SessionUserId is a
- * member of.  We store this mainly so that AtAbort_UserId knows what to
- * reset CurrentUserId to.
+ * member of.  (XXX rename to something like CurrentRoleId?)
  *
  * CurrentUserId is the current effective user ID; this is the one to use
  * for all normal permissions-checking purposes.  At outer level this will
  * be the same as OuterUserId, but it changes during calls to SECURITY
  * DEFINER functions, as well as locally in some specialized commands.
+ *
+ * SecurityDefinerContext is TRUE if we are within a SECURITY DEFINER function
+ * or another context that temporarily changes CurrentUserId.
  * ----------------------------------------------------------------
  */
 static Oid	AuthenticatedUserId = InvalidOid;
@@ -282,12 +284,16 @@ static Oid	CurrentUserId = InvalidOid;
 static bool AuthenticatedUserIsSuperuser = false;
 static bool SessionUserIsSuperuser = false;
 
+static bool SecurityDefinerContext = false;
+
 /* We also remember if a SET ROLE is currently active */
 static bool SetRoleIsActive = false;
 
 
 /*
- * GetUserId/SetUserId - get/set the current effective user ID.
+ * GetUserId - get the current effective user ID.
+ *
+ * Note: there's no SetUserId() anymore; use SetUserIdAndContext().
  */
 Oid
 GetUserId(void)
@@ -297,14 +303,6 @@ GetUserId(void)
 }
 
 
-void
-SetUserId(Oid userid)
-{
-	AssertArg(OidIsValid(userid));
-	CurrentUserId = userid;
-}
-
-
 /*
  * GetOuterUserId/SetOuterUserId - get/set the outer-level user ID.
  */
@@ -319,6 +317,7 @@ GetOuterUserId(void)
 static void
 SetOuterUserId(Oid userid)
 {
+	AssertState(!SecurityDefinerContext);
 	AssertArg(OidIsValid(userid));
 	OuterUserId = userid;
 
@@ -341,6 +340,7 @@ GetSessionUserId(void)
 static void
 SetSessionUserId(Oid userid, bool is_superuser)
 {
+	AssertState(!SecurityDefinerContext);
 	AssertArg(OidIsValid(userid));
 	SessionUserId = userid;
 	SessionUserIsSuperuser = is_superuser;
@@ -352,6 +352,44 @@ SetSessionUserId(Oid userid, bool is_superuser)
 }
 
 
+/*
+ * GetUserIdAndContext/SetUserIdAndContext - get/set the current user ID
+ * and the SecurityDefinerContext flag.
+ *
+ * Unlike GetUserId, GetUserIdAndContext does *not* Assert that the current
+ * value of CurrentUserId is valid; nor does SetUserIdAndContext require
+ * the new value to be valid.  In fact, these routines had better not
+ * ever throw any kind of error.  This is because they are used by
+ * StartTransaction and AbortTransaction to save/restore the settings,
+ * and during the first transaction within a backend, the value to be saved
+ * and perhaps restored is indeed invalid.  We have to be able to get
+ * through AbortTransaction without asserting in case InitPostgres fails.
+ */
+void
+GetUserIdAndContext(Oid *userid, bool *sec_def_context)
+{
+	*userid = CurrentUserId;
+	*sec_def_context = SecurityDefinerContext;
+}
+
+void
+SetUserIdAndContext(Oid userid, bool sec_def_context)
+{
+	CurrentUserId = userid;
+	SecurityDefinerContext = sec_def_context;
+}
+
+
+/*
+ * InSecurityDefinerContext - are we inside a SECURITY DEFINER context?
+ */
+bool
+InSecurityDefinerContext(void)
+{
+	return SecurityDefinerContext;
+}
+
+
 /*
  * Initialize user identity during normal backend startup
  */
@@ -479,21 +517,6 @@ InitializeSessionUserIdStandalone(void)
 }
 
 
-/*
- * Reset effective userid during AbortTransaction
- *
- * This is essentially SetUserId(GetOuterUserId()), but without the Asserts.
- * The reason is that if a backend's InitPostgres transaction fails (eg,
- * because an invalid user name was given), we have to be able to get through
- * AbortTransaction without asserting.
- */
-void
-AtAbort_UserId(void)
-{
-	CurrentUserId = OuterUserId;
-}
-
-
 /*
  * Change session auth ID while running
  *