10000 Make standard maintenance operations (including VACUUM, ANALYZE, REIN… · commandprompt/postgres@218cf59 · GitHub
[go: up one dir, main page]
Skip to content

Commit 218cf59

Browse files
tglsfdctglsfdc
committed
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
1 parent 7146fab commit 218cf59

File tree

10 files changed

+209
-92
lines changed

10 files changed

+209
-92
lines changed

‎doc/src/sgml/ref/set_session_auth.sgml

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
<!-- $Header: /cvsroot/pgsql/doc/src/sgml/ref/set_session_auth.sgml,v 1.7 2002/09/21 18:32:54 petere Exp $ -->
1+
<!-- $Header: /cvsroot/pgsql/doc/src/sgml/ref/set_session_auth.sgml,v 1.7.2.1 2008/01/03 21:25:58 tgl Exp $ -->
22
<refentry id="SQL-SET-SESSION-AUTHORIZATION">
33
<docinfo>
44
<date>2001-04-21</date>
@@ -27,7 +27,7 @@ RESET SESSION AUTHORIZATION
2727

2828
<para>
2929
This command sets the session user identifier and the current user
30-
identifier of the current SQL-session context to be
30+
identifier of the current SQL session to be
3131
<parameter>username</parameter>. The user name may be written as
3232
either an identifier or a string literal.
3333
The session user identifier is valid for the duration of a
@@ -39,7 +39,7 @@ RESET SESSION AUTHORIZATION
3939
The session user identifier is initially set to be the (possibly
4040
authenticated) user name provided by the client. The current user
4141
identifier is normally equal to the session user identifier, but
42-
may change temporarily in the context of <quote>setuid</quote>
42+
might change temporarily in the context of <literal>SECURITY DEFINER</>
4343
functions and similar mechanisms. The current user identifier is
4444
relevant for permission checking.
4545
</para>
@@ -65,6 +65,15 @@ RESET SESSION AUTHORIZATION
6565

6666
</refsect1>
6767

68+
<refsect1>
69+
<title>Notes</title>
70+
71+
<para>
72+
<command>SET SESSION AUTHORIZATION</> cannot be used within a
73+
<literal>SECURITY DEFINER</> function.
74+
</para>
75+
</refsect1>
76+
6877
<refsect1>
6978
<title>Examples</title>
7079

‎src/backend/access/transam/xact.c

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $Header: /cvsroot/pgsql/src/backend/access/transam/xact.c,v 1.135.2.4 2007/04/26 23:25:48 tgl Exp $
11+
* $Header: /cvsroot/pgsql/src/backend/access/transam/xact.c,v 1.135.2.5 2008/01/03 21:25:58 tgl Exp $
1212
*
1313
* NOTES
1414
* Transaction aborts can now occur two ways:
@@ -215,6 +215,8 @@ static TransactionStateData CurrentTransactionStateData = {
215215

216216
TransactionState CurrentTransactionState = &CurrentTransactionStateData;
217217

218+
static Oid prevUser; /* CurrentUserId at transaction start */
219+
218220
/*
219221
* User-tweakable parameters
220222
*/
@@ -960,6 +962,7 @@ static void
960962
CommitTransaction(void)
961963
{
962964
TransactionState s = CurrentTransactionState;
965+
bool prevSecDefCxt;
963966

964967
/*
965968
* check the current transaction state
@@ -983,6 +986,10 @@ CommitTransaction(void)
983986
*/
984987
s->state = TRANS_COMMIT;
985988

989+
GetUserIdAndContext(&prevUser, &prevSecDefCxt);
990+
/* SecurityDefinerContext should never be set outside a transaction */
991+
Assert(!prevSecDefCxt);
992+
986993
/*
987994
* Do pre-commit processing (most of this stuff requires database
988995
* access, and in fact could still cause an error...)
@@ -1118,9 +1125,16 @@ AbortTransaction(void)
11181125
AtAbort_Memory();
11191126

11201127
/*
1121-
* Reset user id which might have been changed transiently
1128+
* Reset user ID which might have been changed transiently. We need this
1129+
* to clean up in case control escaped out of a SECURITY DEFINER function
1130+
* or other local change of CurrentUserId; therefore, the prior value
1131+
* of SecurityDefinerContext also needs to be restored.
1132+
*
1133+
* (Note: it is not necessary to restore session authorization
1134+
* setting here because that can only be changed via GUC, and GUC will
1135+
* take care of rolling it back if need be.)
11221136
*/
1123-
SetUserId(GetSessionUserId());
1137+
SetUserIdAndContext(prevUser, false);
11241138

11251139
/*
11261140
* do abort processing

‎src/backend/catalog/index.c

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $Header: /cvsroot/pgsql/src/backend/catalog/index.c,v 1.202.2.1 2005/06/25 16:54:30 tgl Exp $
11+
* $Header: /cvsroot/pgsql/src/backend/catalog/index.c,v 1.202.2.2 2008/01/03 21:25:58 tgl Exp $
1212
*
1313
*
1414
* INTERFACE ROUTINES
@@ -1439,6 +1439,8 @@ index_build(Relation heapRelation,
14391439
IndexInfo *indexInfo)
14401440
{
14411441
RegProcedure procedure;
1442+
Oid save_userid;
1443+
bool save_secdefcxt;
14421444

14431445
/*
14441446
* sanity checks
@@ -1449,13 +1451,23 @@ index_build(Relation heapRelation,
14491451
procedure = indexRelation->rd_am->ambuild;
14501452
Assert(RegProcedureIsValid(procedure));
14511453

1454+
/*
1455+
* Switch to the table owner's userid, so that any index functions are
1456+
* run as that user.
1457+
*/
1458+
GetUserIdAndContext(&save_userid, &save_secdefcxt);
1459+
SetUserIdAndContext(heapRelation->rd_rel->relowner, true);
1460+
14521461
/*
14531462
* Call the access method's build procedure
14541463
*/
14551464
OidFunctionCall3(procedure,
14561465
PointerGetDatum(heapRelation),
14571466
PointerGetDatum(indexRelation),
14581467
PointerGetDatum(indexInfo));
1468+
1469+
/* Restore userid */
1470+
SetUserIdAndContext(save_userid, save_secdefcxt);
14591471
}
14601472

14611473

‎src/backend/commands/schemacmds.c

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $Header: /cvsroot/pgsql/src/backend/commands/schemacmds.c,v 1.6 2002/09/04 20:31:15 momjian Exp $
11+
* $Header: /cvsroot/pgsql/src/backend/commands/schemacmds.c,v 1.6.2.1 2008/01/03 21:25:58 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -43,9 +43,10 @@ CreateSchemaCommand(CreateSchemaStmt *stmt)
4343
const char *owner_name;
4444
Oid owner_userid;
4545
Oid saved_userid;
46+
bool saved_secdefcxt;
4647
AclResult aclresult;
4748

48-
saved_userid = GetUserId();
49+
GetUserIdAndContext(&saved_userid, &saved_secdefcxt);
4950

5051
/*
5152
* Figure out user identities.
@@ -68,7 +69,7 @@ CreateSchemaCommand(CreateSchemaStmt *stmt)
6869
* (This will revert to session user on error or at the end of
6970
* this routine.)
7071
*/
71-
SetUserId(owner_userid);
72+
SetUserIdAndContext(owner_userid, true);
7273
}
7374
else
7475
/* not superuser */
@@ -143,7 +144,7 @@ CreateSchemaCommand(CreateSchemaStmt *stmt)
143144
PopSpecialNamespace(namespaceId);
144145

145146
/* Reset current user */
146-
SetUserId(saved_userid);
147+
SetUserIdAndContext(saved_userid, saved_secdefcxt);
147148
}
148149

149150

‎src/backend/commands/vacuum.c

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
*
1414
*
1515
* IDENTIFICATION
16-
* $Header: /cvsroot/pgsql/src/backend/commands/vacuum.c,v 1.244.2.2 2007/03/14 18:49:32 tgl Exp $
16+
* $Header: /cvsroot/pgsql/src/backend/commands/vacuum.c,v 1.244.2.3 2008/01/03 21:25:58 tgl Exp $
1717
*
1818
*-------------------------------------------------------------------------
1919
*/
@@ -720,6 +720,8 @@ vacuum_rel(Oid relid, VacuumStmt *vacstmt, char expected_relkind)
720720
LockRelId onerelid;
721721
Oid toast_relid;
722722
bool result;
723+
Oid save_userid;
724+
bool save_secdefcxt;
723725

724726
/* Begin a transaction for vacuuming this relation */
725727
StartTransactionCommand(true);
@@ -820,6 +822,14 @@ vacuum_rel(Oid relid, VacuumStmt *vacstmt, char expected_relkind)
820822
*/
821823
toast_relid = onerel->rd_rel->reltoastrelid;
822824

825+
/*
826+
* Switch to the table owner's userid, so that any index functions are
827+
* run as that user. (This is unnecessary, but harmless, for lazy
828+
* VACUUM.)
829+
*/
830+
GetUserIdAndContext(&save_userid, &save_secdefcxt);
831+
SetUserIdAndContext(onerel->rd_rel->relowner, true);
832+
823833
/*
824834
* Do the actual work --- either FULL or "lazy" vacuum
825835
*/
@@ -830,6 +840,9 @@ vacuum_rel(Oid relid, VacuumStmt *vacstmt, char expected_relkind)
830840

831841
result = true; /* did the vacuum */
832842

843+
/* Restore userid */
844+
SetUserIdAndContext(save_userid, save_secdefcxt);
845+
833846
/* all done with this class, but hold lock until commit */
834847
relation_close(onerel, NoLock);
835848

‎src/backend/commands/variable.c

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
*
1010
*
1111
* IDENTIFICATION
12-
* $Header: /cvsroot/pgsql/src/backend/commands/variable.c,v 1.71.2.4 2006/02/12 22:33:46 tgl Exp $
12+
* $Header: /cvsroot/pgsql/src/backend/commands/variable.c,v 1.71.2.5 2008/01/03 21:25:58 tgl Exp $
1313
*
1414
*-------------------------------------------------------------------------
1515
*/
@@ -561,6 +561,20 @@ assign_session_authorization(const char *value, bool doit, bool interactive)
561561
/* not a saved ID, so look it up */
562562
HeapTuple userTup;
563563

564+
if (InSecurityDefinerContext())
565+
{
566+
/*
567+
* Disallow SET SESSION AUTHORIZATION inside a security definer
568+
* context. We need to do this because when we exit the context,
569+
* GUC won't be notified, leaving things out of sync. Note that
570+
* this test is positioned so that restoring a previously saved
571+
* setting isn't prevented.
572+
*/
573+
if (interactive)
574+
elog(ERROR, "cannot set session authorization within security-definer function");
575+
return NULL;
576+
}
577+
564578
if (! IsTransactionState())
565579
{
566580
/*

0 commit comments

Comments
 (0)
0