Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit62aba76

Browse files
committed
Prevent indirect security attacks via changing session-local state within
an allegedly immutable index function. It was previously recognized thatwe had to prevent such a function from executing SET/RESET ROLE/SESSIONAUTHORIZATION, or it could trivially obtain the privileges of the sessionuser. However, since there is in general no privilege checking for changesof session-local state, it is also possible for such a function to changesettings in a way that might subvert later operations in the same session.Examples include changing search_path to cause an unexpected function tobe called, or replacing an existing prepared statement with another onethat will execute a function of the attacker's choosing.The present patch secures VACUUM, ANALYZE, and CREATE INDEX/REINDEX againstthese threats, which are the same places previously deemed to need protectionagainst the SET ROLE issue. GUC changes are still allowed, since there aremany useful cases for that, but we prevent security problems by forcing arollback of any GUC change after completing the operation. Other cases arehandled by throwing an error if any change is attempted; these include temptable creation, closing a cursor, and creating or deleting a preparedstatement. (In 7.4, the infrastructure to roll back GUC changes doesn'texist, so we settle for rejecting changes of "search_path" in these contexts.)Original report and patch by Gurjeet Singh, additional analysis byTom Lane.Security:CVE-2009-4136
1 parent7aeaa97 commit62aba76

File tree

14 files changed

+273
-104
lines changed

14 files changed

+273
-104
lines changed

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

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
*
1111
*
1212
* IDENTIFICATION
13-
* $PostgreSQL: pgsql/src/backend/access/transam/xact.c,v 1.276 2009/11/23 09:58:36 heikki Exp $
13+
* $PostgreSQL: pgsql/src/backend/access/transam/xact.c,v 1.277 2009/12/09 21:57:50 tgl Exp $
1414
*
1515
*-------------------------------------------------------------------------
1616
*/
@@ -137,7 +137,7 @@ typedef struct TransactionStateData
137137
intnChildXids;/* # of subcommitted child XIDs */
138138
intmaxChildXids;/* allocated size of childXids[] */
139139
OidprevUser;/* previous CurrentUserId setting */
140-
boolprevSecDefCxt;/* previousSecurityDefinerContext setting */
140+
intprevSecContext;/* previousSecurityRestrictionContext */
141141
boolprevXactReadOnly;/* entry-time xact r/o state */
142142
structTransactionStateData*parent;/* back link to parent */
143143
}TransactionStateData;
@@ -165,7 +165,7 @@ static TransactionStateData TopTransactionStateData = {
165165
0,/* # of subcommitted child Xids */
166166
0,/* allocated size of childXids[] */
167167
InvalidOid,/* previous CurrentUserId setting */
168-
false,/* previousSecurityDefinerContext setting */
168+
0,/* previousSecurityRestrictionContext */
169169
false,/* entry-time xact r/o state */
170170
NULL/* link to parent state block */
171171
};
@@ -1522,9 +1522,9 @@ StartTransaction(void)
15221522
s->childXids=NULL;
15231523
s->nChildXids=0;
15241524
s->maxChildXids=0;
1525-
GetUserIdAndContext(&s->prevUser,&s->prevSecDefCxt);
1526-
/*SecurityDefinerContext should never be set outside a transaction */
1527-
Assert(!s->prevSecDefCxt);
1525+
GetUserIdAndSecContext(&s->prevUser,&s->prevSecContext);
1526+
/*SecurityRestrictionContext should never be set outside a transaction */
1527+
Assert(s->prevSecContext==0);
15281528

15291529
/*
15301530
* initialize other subsystems for new transaction
@@ -2014,13 +2014,13 @@ AbortTransaction(void)
20142014
* Reset user ID which might have been changed transiently. We need this
20152015
* to clean up in case control escaped out of a SECURITY DEFINER function
20162016
* or other local change of CurrentUserId; therefore, the prior value of
2017-
*SecurityDefinerContext also needs to be restored.
2017+
*SecurityRestrictionContext also needs to be restored.
20182018
*
20192019
* (Note: it is not necessary to restore session authorization or role
20202020
* settings here because those can only be changed via GUC, and GUC will
20212021
* take care of rolling them back if need be.)
20222022
*/
2023-
SetUserIdAndContext(s->prevUser,s->prevSecDefCxt);
2023+
SetUserIdAndSecContext(s->prevUser,s->prevSecContext);
20242024

20252025
/*
20262026
* do abort processing
@@ -3860,7 +3860,7 @@ AbortSubTransaction(void)
38603860
* Reset user ID which might have been changed transiently. (See notes in
38613861
* AbortTransaction.)
38623862
*/
3863-
SetUserIdAndContext(s->prevUser,s->prevSecDefCxt);
3863+
SetUserIdAndSecContext(s->prevUser,s->prevSecContext);
38643864

38653865
/*
38663866
* We can skip all this stuff if the subxact failed before creating a
@@ -4000,7 +4000,7 @@ PushTransaction(void)
40004000
s->savepointLevel=p->savepointLevel;
40014001
s->state=TRANS_DEFAULT;
40024002
s->blockState=TBLOCK_SUBBEGIN;
4003-
GetUserIdAndContext(&s->prevUser,&s->prevSecDefCxt);
4003+
GetUserIdAndSecContext(&s->prevUser,&s->prevSecContext);
40044004
s->prevXactReadOnly=XactReadOnly;
40054005

40064006
CurrentTransactionState=s;

‎src/backend/catalog/index.c

Lines changed: 28 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/catalog/index.c,v 1.325 2009/12/07 05:22:21 tgl Exp $
11+
* $PostgreSQL: pgsql/src/backend/catalog/index.c,v 1.326 2009/12/09 21:57:50 tgl Exp $
1212
*
1313
*
1414
* INTERFACE ROUTINES
@@ -58,6 +58,7 @@
5858
#include"storage/smgr.h"
5959
#include"utils/builtins.h"
6060
#include"utils/fmgroids.h"
61+
#include"utils/guc.h"
6162
#include"utils/inval.h"
6263
#include"utils/lsyscache.h"
6364
#include"utils/memutils.h"
@@ -1481,7 +1482,8 @@ index_build(Relation heapRelation,
14811482
RegProcedureprocedure;
14821483
IndexBuildResult*stats;
14831484
Oidsave_userid;
1484-
boolsave_secdefcxt;
1485+
intsave_sec_context;
1486+
intsave_nestlevel;
14851487

14861488
/*
14871489
* sanity checks
@@ -1494,10 +1496,13 @@ index_build(Relation heapRelation,
14941496

14951497
/*
14961498
* Switch to the table owner's userid, so that any index functions are run
1497-
* as that user.
1499+
* as that user. Also lock down security-restricted operations and
1500+
* arrange to make GUC variable changes local to this command.
14981501
*/
1499-
GetUserIdAndContext(&save_userid,&save_secdefcxt);
1500-
SetUserIdAndContext(heapRelation->rd_rel->relowner, true);
1502+
GetUserIdAndSecContext(&save_userid,&save_sec_context);
1503+
SetUserIdAndSecContext(heapRelation->rd_rel->relowner,
1504+
save_sec_context |SECURITY_RESTRICTED_OPERATION);
1505+
save_nestlevel=NewGUCNestLevel();
15011506

15021507
/*
15031508
* Call the access method's build procedure
@@ -1516,8 +1521,11 @@ index_build(Relation heapRelation,
15161521
if (indexInfo->ii_ExclusionOps!=NULL)
15171522
IndexCheckExclusion(heapRelation,indexRelation,indexInfo);
15181523

1519-
/* Restore userid */
1520-
SetUserIdAndContext(save_userid,save_secdefcxt);
1524+
/* Roll back any GUC changes executed by index functions */
1525+
AtEOXact_GUC(false,save_nestlevel);
1526+
1527+
/* Restore userid and security context */
1528+
SetUserIdAndSecContext(save_userid,save_sec_context);
15211529

15221530
/*
15231531
* If we found any potentially broken HOT chains, mark the index as not
@@ -2126,7 +2134,8 @@ validate_index(Oid heapId, Oid indexId, Snapshot snapshot)
21262134
IndexVacuumInfoivinfo;
21272135
v_i_statestate;
21282136
Oidsave_userid;
2129-
boolsave_secdefcxt;
2137+
intsave_sec_context;
2138+
intsave_nestlevel;
21302139

21312140
/* Open and lock the parent heap relation */
21322141
heapRelation=heap_open(heapId,ShareUpdateExclusiveLock);
@@ -2145,10 +2154,13 @@ validate_index(Oid heapId, Oid indexId, Snapshot snapshot)
21452154

21462155
/*
21472156
* Switch to the table owner's userid, so that any index functions are run
2148-
* as that user.
2157+
* as that user. Also lock down security-restricted operations and
2158+
* arrange to make GUC variable changes local to this command.
21492159
*/
2150-
GetUserIdAndContext(&save_userid,&save_secdefcxt);
2151-
SetUserIdAndContext(heapRelation->rd_rel->relowner, true);
2160+
GetUserIdAndSecContext(&save_userid,&save_sec_context);
2161+
SetUserIdAndSecContext(heapRelation->rd_rel->relowner,
2162+
save_sec_context |SECURITY_RESTRICTED_OPERATION);
2163+
save_nestlevel=NewGUCNestLevel();
21522164

21532165
/*
21542166
* Scan the index and gather up all the TIDs into a tuplesort object.
@@ -2189,8 +2201,11 @@ validate_index(Oid heapId, Oid indexId, Snapshot snapshot)
21892201
"validate_index found %.0f heap tuples, %.0f index tuples; inserted %.0f missing tuples",
21902202
state.htups,state.itups,state.tups_inserted);
21912203

2192-
/* Restore userid */
2193-
SetUserIdAndContext(save_userid,save_secdefcxt);
2204+
/* Roll back any GUC changes executed by index functions */
2205+
AtEOXact_GUC(false,save_nestlevel);
2206+
2207+
/* Restore userid and security context */
2208+
SetUserIdAndSecContext(save_userid,save_sec_context);
21942209

21952210
/* Close rels, but keep locks */
21962211
index_close(indexRelation,NoLock);

‎src/backend/commands/analyze.c

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/commands/analyze.c,v 1.142 2009/11/16 21:32:06 tgl Exp $
11+
* $PostgreSQL: pgsql/src/backend/commands/analyze.c,v 1.143 2009/12/09 21:57:50 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -38,6 +38,7 @@
3838
#include"storage/procarray.h"
3939
#include"utils/acl.h"
4040
#include"utils/datum.h"
41+
#include"utils/guc.h"
4142
#include"utils/lsyscache.h"
4243
#include"utils/memutils.h"
4344
#include"utils/pg_rusage.h"
@@ -133,7 +134,8 @@ analyze_rel(Oid relid, VacuumStmt *vacstmt,
133134
PGRUsageru0;
134135
TimestampTzstarttime=0;
135136
Oidsave_userid;
136-
boolsave_secdefcxt;
137+
intsave_sec_context;
138+
intsave_nestlevel;
137139

138140
if (vacstmt->options&VACOPT_VERBOSE)
139141
elevel=INFO;
@@ -235,10 +237,13 @@ analyze_rel(Oid relid, VacuumStmt *vacstmt,
235237

236238
/*
237239
* Switch to the table owner's userid, so that any index functions are run
238-
* as that user.
240+
* as that user. Also lock down security-restricted operations and
241+
* arrange to make GUC variable changes local to this command.
239242
*/
240-
GetUserIdAndContext(&save_userid,&save_secdefcxt);
241-
SetUserIdAndContext(onerel->rd_rel->relowner, true);
243+
GetUserIdAndSecContext(&save_userid,&save_sec_context);
244+
SetUserIdAndSecContext(onerel->rd_rel->relowner,
245+
save_sec_context |SECURITY_RESTRICTED_OPERATION);
246+
save_nestlevel=NewGUCNestLevel();
242247

243248
/* let others know what I'm doing */
244249
LWLockAcquire(ProcArrayLock,LW_EXCLUSIVE);
@@ -548,8 +553,11 @@ analyze_rel(Oid relid, VacuumStmt *vacstmt,
548553
MyProc->vacuumFlags &= ~PROC_IN_ANALYZE;
549554
LWLockRelease(ProcArrayLock);
550555

551-
/* Restore userid */
552-
SetUserIdAndContext(save_userid,save_secdefcxt);
556+
/* Roll back any GUC changes executed by index functions */
557+
AtEOXact_GUC(false,save_nestlevel);
558+
559+
/* Restore userid and security context */
560+
SetUserIdAndSecContext(save_userid,save_sec_context);
553561
}
554562

555563
/*

‎src/backend/commands/schemacmds.c

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/commands/schemacmds.c,v 1.53 2009/06/11 14:48:56 momjian Exp $
11+
* $PostgreSQL: pgsql/src/backend/commands/schemacmds.c,v 1.54 2009/12/09 21:57:50 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -48,10 +48,10 @@ CreateSchemaCommand(CreateSchemaStmt *stmt, const char *queryString)
4848
ListCell*parsetree_item;
4949
Oidowner_uid;
5050
Oidsaved_uid;
51-
boolsaved_secdefcxt;
51+
intsave_sec_context;
5252
AclResultaclresult;
5353

54-
GetUserIdAndContext(&saved_uid,&saved_secdefcxt);
54+
GetUserIdAndSecContext(&saved_uid,&save_sec_context);
5555

5656
/*
5757
* Who is supposed to own the new schema?
@@ -91,7 +91,8 @@ CreateSchemaCommand(CreateSchemaStmt *stmt, const char *queryString)
9191
* error, transaction abort will clean things up.)
9292
*/
9393
if (saved_uid!=owner_uid)
94-
SetUserIdAndContext(owner_uid, true);
94+
SetUserIdAndSecContext(owner_uid,
95+
save_sec_context |SECURITY_LOCAL_USERID_CHANGE);
9596

9697
/* Create the schema's namespace */
9798
namespaceId=NamespaceCreate(schemaName,owner_uid);
@@ -142,8 +143,8 @@ CreateSchemaCommand(CreateSchemaStmt *stmt, const char *queryString)
142143
/* Reset search path to normal state */
143144
PopOverrideSearchPath();
144145

145-
/* Reset current user */
146-
SetUserIdAndContext(saved_uid,saved_secdefcxt);
146+
/* Reset current userand security context*/
147+
SetUserIdAndSecContext(saved_uid,save_sec_context);
147148
}
148149

149150

‎src/backend/commands/tablecmds.c

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/commands/tablecmds.c,v 1.307 2009/12/07 05:22:21 tgl Exp $
11+
* $PostgreSQL: pgsql/src/backend/commands/tablecmds.c,v 1.308 2009/12/09 21:57:50 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -378,6 +378,16 @@ DefineRelation(CreateStmt *stmt, char relkind)
378378
(errcode(ERRCODE_INVALID_TABLE_DEFINITION),
379379
errmsg("ON COMMIT can only be used on temporary tables")));
380380

381+
/*
382+
* Security check: disallow creating temp tables from security-restricted
383+
* code. This is needed because calling code might not expect untrusted
384+
* tables to appear in pg_temp at the front of its search path.
385+
*/
386+
if (stmt->relation->istemp&&InSecurityRestrictedOperation())
387+
ereport(ERROR,
388+
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
389+
errmsg("cannot create temporary table within security-restricted operation")));
390+
381391
/*
382392
* Look up the namespace in which we are supposed to create the relation.
383393
* Check we have permission to create there. Skip check if bootstrapping,

‎src/backend/commands/vacuum.c

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
*
1414
*
1515
* IDENTIFICATION
16-
* $PostgreSQL: pgsql/src/backend/commands/vacuum.c,v 1.397 2009/12/07 05:22:21 tgl Exp $
16+
* $PostgreSQL: pgsql/src/backend/commands/vacuum.c,v 1.398 2009/12/09 21:57:51 tgl Exp $
1717
*
1818
*-------------------------------------------------------------------------
1919
*/
@@ -47,6 +47,7 @@
4747
#include"utils/acl.h"
4848
#include"utils/builtins.h"
4949
#include"utils/fmgroids.h"
50+
#include"utils/guc.h"
5051
#include"utils/inval.h"
5152
#include"utils/lsyscache.h"
5253
#include"utils/memutils.h"
@@ -1033,7 +1034,8 @@ vacuum_rel(Oid relid, VacuumStmt *vacstmt, bool do_toast, bool for_wraparound,
10331034
LockRelIdonerelid;
10341035
Oidtoast_relid;
10351036
Oidsave_userid;
1036-
boolsave_secdefcxt;
1037+
intsave_sec_context;
1038+
intsave_nestlevel;
10371039
boolheldoff;
10381040

10391041
if (scanned_all)
@@ -1192,10 +1194,14 @@ vacuum_rel(Oid relid, VacuumStmt *vacstmt, bool do_toast, bool for_wraparound,
11921194

11931195
/*
11941196
* Switch to the table owner's userid, so that any index functions are run
1195-
* as that user. (This is unnecessary, but harmless, for lazy VACUUM.)
1197+
* as that user. Also lock down security-restricted operations and
1198+
* arrange to make GUC variable changes local to this command.
1199+
* (This is unnecessary, but harmless, for lazy VACUUM.)
11961200
*/
1197-
GetUserIdAndContext(&save_userid,&save_secdefcxt);
1198-
SetUserIdAndContext(onerel->rd_rel->relowner, true);
1201+
GetUserIdAndSecContext(&save_userid,&save_sec_context);
1202+
SetUserIdAndSecContext(onerel->rd_rel->relowner,
1203+
save_sec_context |SECURITY_RESTRICTED_OPERATION);
1204+
save_nestlevel=NewGUCNestLevel();
11991205

12001206
/*
12011207
* Do the actual work --- either FULL or "lazy" vacuum
@@ -1205,8 +1211,11 @@ vacuum_rel(Oid relid, VacuumStmt *vacstmt, bool do_toast, bool for_wraparound,
12051211
else
12061212
heldoff=lazy_vacuum_rel(onerel,vacstmt,vac_strategy,scanned_all);
12071213

1208-
/* Restore userid */
1209-
SetUserIdAndContext(save_userid,save_secdefcxt);
1214+
/* Roll back any GUC changes executed by index functions */
1215+
AtEOXact_GUC(false,save_nestlevel);
1216+
1217+
/* Restore userid and security context */
1218+
SetUserIdAndSecContext(save_userid,save_sec_context);
12101219

12111220
/* all done with this class, but hold lock until commit */
12121221
relation_close(onerel,NoLock);

‎src/backend/executor/execMain.c

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@
2626
*
2727
*
2828
* IDENTIFICATION
29-
* $PostgreSQL: pgsql/src/backend/executor/execMain.c,v 1.335 2009/11/20 20:38:10 tgl Exp $
29+
* $PostgreSQL: pgsql/src/backend/executor/execMain.c,v 1.336 2009/12/09 21:57:51 tgl Exp $
3030
*
3131
*-------------------------------------------------------------------------
3232
*/
@@ -2067,6 +2067,11 @@ OpenIntoRel(QueryDesc *queryDesc)
20672067

20682068
Assert(into);
20692069

2070+
/*
2071+
* XXX This code needs to be kept in sync with DefineRelation().
2072+
* Maybe we should try to use that function instead.
2073+
*/
2074+
20702075
/*
20712076
* Check consistency of arguments
20722077
*/
@@ -2075,6 +2080,16 @@ OpenIntoRel(QueryDesc *queryDesc)
20752080
(errcode(ERRCODE_INVALID_TABLE_DEFINITION),
20762081
errmsg("ON COMMIT can only be used on temporary tables")));
20772082

2083+
/*
2084+
* Security check: disallow creating temp tables from security-restricted
2085+
* code. This is needed because calling code might not expect untrusted
2086+
* tables to appear in pg_temp at the front of its search path.
2087+
*/
2088+
if (into->rel->istemp&&InSecurityRestrictedOperation())
2089+
ereport(ERROR,
2090+
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
2091+
errmsg("cannot create temporary table within security-restricted operation")));
2092+
20782093
/*
20792094
* Find namespace to create in, check its permissions
20802095
*/

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp