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

Commitab49ce7

Browse files
committed
Make relation-enumerating operations be security-restricted operations.
When a feature enumerates relations and runs functions associated withall found relations, the feature's user shall not need to trust everyuser having permission to create objects. BRIN-specific functionalityin autovacuum neglected to account for this, as did pg_amcheck andCLUSTER. An attacker having permission to create non-temp objects in atleast one schema could execute arbitrary SQL functions under theidentity of the bootstrap superuser. CREATE INDEX (not arelation-enumerating operation) and REINDEX protected themselves toolate. This change extends to the non-enumerating amcheck interface.Back-patch to v10 (all supported versions).Sergey Shinderuk, reviewed (in earlier versions) by Alexander Lakhin.Reported by Alexander Lakhin.Security:CVE-2022-1552
1 parente5b5a21 commitab49ce7

File tree

10 files changed

+378
-48
lines changed

10 files changed

+378
-48
lines changed

‎contrib/amcheck/expected/check_btree.out‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -177,11 +177,34 @@ SELECT bt_index_check('toasty', true);
177177

178178
(1 row)
179179

180+
--
181+
-- Check that index expressions and predicates are run as the table's owner
182+
--
183+
TRUNCATE bttest_a;
184+
INSERT INTO bttest_a SELECT * FROM generate_series(1, 1000);
185+
ALTER TABLE bttest_a OWNER TO regress_bttest_role;
186+
-- A dummy index function checking current_user
187+
CREATE FUNCTION ifun(int8) RETURNS int8 AS $$
188+
BEGIN
189+
ASSERT current_user = 'regress_bttest_role',
190+
format('ifun(%s) called by %s', $1, current_user);
191+
RETURN $1;
192+
END;
193+
$$ LANGUAGE plpgsql IMMUTABLE;
194+
CREATE INDEX bttest_a_expr_idx ON bttest_a ((ifun(id) + ifun(0)))
195+
WHERE ifun(id + 10) > ifun(10);
196+
SELECT bt_index_check('bttest_a_expr_idx', true);
197+
bt_index_check
198+
----------------
199+
200+
(1 row)
201+
180202
-- cleanup
181203
DROP TABLE bttest_a;
182204
DROP TABLE bttest_b;
183205
DROP TABLE bttest_multi;
184206
DROP TABLE delete_test_table;
185207
DROP TABLE toast_bug;
208+
DROP FUNCTION ifun(int8);
186209
DROP OWNED BY regress_bttest_role; -- permissions
187210
DROP ROLE regress_bttest_role;

‎contrib/amcheck/sql/check_btree.sql‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,11 +115,32 @@ INSERT INTO toast_bug SELECT repeat('a', 2200);
115115
-- Should not get false positive report of corruption:
116116
SELECT bt_index_check('toasty', true);
117117

118+
--
119+
-- Check that index expressions and predicates are run as the table's owner
120+
--
121+
TRUNCATE bttest_a;
122+
INSERT INTO bttest_aSELECT*FROM generate_series(1,1000);
123+
ALTERTABLE bttest_a OWNER TO regress_bttest_role;
124+
-- A dummy index function checking current_user
125+
CREATEFUNCTIONifun(int8) RETURNS int8AS $$
126+
BEGIN
127+
ASSERTcurrent_user='regress_bttest_role',
128+
format('ifun(%s) called by %s', $1,current_user);
129+
RETURN $1;
130+
END;
131+
$$ LANGUAGE plpgsql IMMUTABLE;
132+
133+
CREATEINDEXbttest_a_expr_idxON bttest_a ((ifun(id)+ ifun(0)))
134+
WHERE ifun(id+10)> ifun(10);
135+
136+
SELECT bt_index_check('bttest_a_expr_idx', true);
137+
118138
-- cleanup
119139
DROPTABLE bttest_a;
120140
DROPTABLE bttest_b;
121141
DROPTABLE bttest_multi;
122142
DROPTABLE delete_test_table;
123143
DROPTABLE toast_bug;
144+
DROPFUNCTION ifun(int8);
124145
DROP OWNED BY regress_bttest_role;-- permissions
125146
DROP ROLE regress_bttest_role;

‎contrib/amcheck/verify_nbtree.c‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -248,6 +248,9 @@ bt_index_check_internal(Oid indrelid, bool parentcheck, bool heapallindexed,
248248
Relationindrel;
249249
Relationheaprel;
250250
LOCKMODElockmode;
251+
Oidsave_userid;
252+
intsave_sec_context;
253+
intsave_nestlevel;
251254

252255
if (parentcheck)
253256
lockmode=ShareLock;
@@ -264,9 +267,27 @@ bt_index_check_internal(Oid indrelid, bool parentcheck, bool heapallindexed,
264267
*/
265268
heapid=IndexGetRelation(indrelid, true);
266269
if (OidIsValid(heapid))
270+
{
267271
heaprel=table_open(heapid,lockmode);
272+
273+
/*
274+
* Switch to the table owner's userid, so that any index functions are
275+
* run as that user. Also lock down security-restricted operations
276+
* and arrange to make GUC variable changes local to this command.
277+
*/
278+
GetUserIdAndSecContext(&save_userid,&save_sec_context);
279+
SetUserIdAndSecContext(heaprel->rd_rel->relowner,
280+
save_sec_context |SECURITY_RESTRICTED_OPERATION);
281+
save_nestlevel=NewGUCNestLevel();
282+
}
268283
else
284+
{
269285
heaprel=NULL;
286+
/* for "gcc -Og" https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78394 */
287+
save_userid=InvalidOid;
288+
save_sec_context=-1;
289+
save_nestlevel=-1;
290+
}
270291

271292
/*
272293
* Open the target index relations separately (like relation_openrv(), but
@@ -326,6 +347,12 @@ bt_index_check_internal(Oid indrelid, bool parentcheck, bool heapallindexed,
326347
heapallindexed,rootdescend);
327348
}
328349

350+
/* Roll back any GUC changes executed by index functions */
351+
AtEOXact_GUC(false,save_nestlevel);
352+
353+
/* Restore userid and security context */
354+
SetUserIdAndSecContext(save_userid,save_sec_context);
355+
329356
/*
330357
* Release locks early. That's ok here because nothing in the called
331358
* routines will trigger shared cache invalidations to be sent, so we can

‎src/backend/access/brin/brin.c‎

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1007,6 +1007,9 @@ brin_summarize_range(PG_FUNCTION_ARGS)
10071007
Oidheapoid;
10081008
RelationindexRel;
10091009
RelationheapRel;
1010+
Oidsave_userid;
1011+
intsave_sec_context;
1012+
intsave_nestlevel;
10101013
doublenumSummarized=0;
10111014

10121015
if (RecoveryInProgress())
@@ -1033,7 +1036,22 @@ brin_summarize_range(PG_FUNCTION_ARGS)
10331036
*/
10341037
heapoid=IndexGetRelation(indexoid, true);
10351038
if (OidIsValid(heapoid))
1039+
{
10361040
heapRel=table_open(heapoid,ShareUpdateExclusiveLock);
1041+
1042+
/*
1043+
* Autovacuum calls us. For its benefit, switch to the table owner's
1044+
* userid, so that any index functions are run as that user. Also
1045+
* lock down security-restricted operations and arrange to make GUC
1046+
* variable changes local to this command. This is harmless, albeit
1047+
* unnecessary, when called from SQL, because we fail shortly if the
1048+
* user does not own the index.
1049+
*/
1050+
GetUserIdAndSecContext(&save_userid,&save_sec_context);
1051+
SetUserIdAndSecContext(heapRel->rd_rel->relowner,
1052+
save_sec_context |SECURITY_RESTRICTED_OPERATION);
1053+
save_nestlevel=NewGUCNestLevel();
1054+
}
10371055
else
10381056
heapRel=NULL;
10391057

@@ -1048,7 +1066,7 @@ brin_summarize_range(PG_FUNCTION_ARGS)
10481066
RelationGetRelationName(indexRel))));
10491067

10501068
/* User must own the index (comparable to privileges needed for VACUUM) */
1051-
if (!pg_class_ownercheck(indexoid,GetUserId()))
1069+
if (heapRel!=NULL&& !pg_class_ownercheck(indexoid,save_userid))
10521070
aclcheck_error(ACLCHECK_NOT_OWNER,OBJECT_INDEX,
10531071
RelationGetRelationName(indexRel));
10541072

@@ -1066,6 +1084,12 @@ brin_summarize_range(PG_FUNCTION_ARGS)
10661084
/* OK, do it */
10671085
brinsummarize(indexRel,heapRel,heapBlk, true,&numSummarized,NULL);
10681086

1087+
/* Roll back any GUC changes executed by index functions */
1088+
AtEOXact_GUC(false,save_nestlevel);
1089+
1090+
/* Restore userid and security context */
1091+
SetUserIdAndSecContext(save_userid,save_sec_context);
1092+
10691093
relation_close(indexRel,ShareUpdateExclusiveLock);
10701094
relation_close(heapRel,ShareUpdateExclusiveLock);
10711095

@@ -1107,6 +1131,9 @@ brin_desummarize_range(PG_FUNCTION_ARGS)
11071131
* passed indexoid isn't an index then IndexGetRelation() will fail.
11081132
* Rather than emitting a not-very-helpful error message, postpone
11091133
* complaining, expecting that the is-it-an-index test below will fail.
1134+
*
1135+
* Unlike brin_summarize_range(), autovacuum never calls this. Hence, we
1136+
* don't switch userid.
11101137
*/
11111138
heapoid=IndexGetRelation(indexoid, true);
11121139
if (OidIsValid(heapoid))

‎src/backend/catalog/index.c‎

Lines changed: 51 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1424,6 +1424,9 @@ index_concurrently_build(Oid heapRelationId,
14241424
OidindexRelationId)
14251425
{
14261426
RelationheapRel;
1427+
Oidsave_userid;
1428+
intsave_sec_context;
1429+
intsave_nestlevel;
14271430
RelationindexRelation;
14281431
IndexInfo*indexInfo;
14291432

@@ -1433,7 +1436,16 @@ index_concurrently_build(Oid heapRelationId,
14331436
/* Open and lock the parent heap relation */
14341437
heapRel=table_open(heapRelationId,ShareUpdateExclusiveLock);
14351438

1436-
/* And the target index relation */
1439+
/*
1440+
* Switch to the table owner's userid, so that any index functions are run
1441+
* as that user. Also lock down security-restricted operations and
1442+
* arrange to make GUC variable changes local to this command.
1443+
*/
1444+
GetUserIdAndSecContext(&save_userid,&save_sec_context);
1445+
SetUserIdAndSecContext(heapRel->rd_rel->relowner,
1446+
save_sec_context |SECURITY_RESTRICTED_OPERATION);
1447+
save_nestlevel=NewGUCNestLevel();
1448+
14371449
indexRelation=index_open(indexRelationId,RowExclusiveLock);
14381450

14391451
/*
@@ -1449,6 +1461,12 @@ index_concurrently_build(Oid heapRelationId,
14491461
/* Now build the index */
14501462
index_build(heapRel,indexRelation,indexInfo, false, true);
14511463

1464+
/* Roll back any GUC changes executed by index functions */
1465+
AtEOXact_GUC(false,save_nestlevel);
1466+
1467+
/* Restore userid and security context */
1468+
SetUserIdAndSecContext(save_userid,save_sec_context);
1469+
14521470
/* Close both the relations, but keep the locks */
14531471
table_close(heapRel,NoLock);
14541472
index_close(indexRelation,NoLock);
@@ -3294,7 +3312,17 @@ validate_index(Oid heapId, Oid indexId, Snapshot snapshot)
32943312

32953313
/* Open and lock the parent heap relation */
32963314
heapRelation=table_open(heapId,ShareUpdateExclusiveLock);
3297-
/* And the target index relation */
3315+
3316+
/*
3317+
* Switch to the table owner's userid, so that any index functions are run
3318+
* as that user. Also lock down security-restricted operations and
3319+
* arrange to make GUC variable changes local to this command.
3320+
*/
3321+
GetUserIdAndSecContext(&save_userid,&save_sec_context);
3322+
SetUserIdAndSecContext(heapRelation->rd_rel->relowner,
3323+
save_sec_context |SECURITY_RESTRICTED_OPERATION);
3324+
save_nestlevel=NewGUCNestLevel();
3325+
32983326
indexRelation=index_open(indexId,RowExclusiveLock);
32993327

33003328
/*
@@ -3307,16 +3335,6 @@ validate_index(Oid heapId, Oid indexId, Snapshot snapshot)
33073335
/* mark build is concurrent just for consistency */
33083336
indexInfo->ii_Concurrent= true;
33093337

3310-
/*
3311-
* Switch to the table owner's userid, so that any index functions are run
3312-
* as that user. Also lock down security-restricted operations and
3313-
* arrange to make GUC variable changes local to this command.
3314-
*/
3315-
GetUserIdAndSecContext(&save_userid,&save_sec_context);
3316-
SetUserIdAndSecContext(heapRelation->rd_rel->relowner,
3317-
save_sec_context |SECURITY_RESTRICTED_OPERATION);
3318-
save_nestlevel=NewGUCNestLevel();
3319-
33203338
/*
33213339
* Scan the index and gather up all the TIDs into a tuplesort object.
33223340
*/
@@ -3525,6 +3543,9 @@ reindex_index(Oid indexId, bool skip_constraint_checks, char persistence,
35253543
RelationiRel,
35263544
heapRelation;
35273545
OidheapId;
3546+
Oidsave_userid;
3547+
intsave_sec_context;
3548+
intsave_nestlevel;
35283549
IndexInfo*indexInfo;
35293550
volatileboolskipped_constraint= false;
35303551
PGRUsageru0;
@@ -3552,6 +3573,16 @@ reindex_index(Oid indexId, bool skip_constraint_checks, char persistence,
35523573
if (!heapRelation)
35533574
return;
35543575

3576+
/*
3577+
* Switch to the table owner's userid, so that any index functions are run
3578+
* as that user. Also lock down security-restricted operations and
3579+
* arrange to make GUC variable changes local to this command.
3580+
*/
3581+
GetUserIdAndSecContext(&save_userid,&save_sec_context);
3582+
SetUserIdAndSecContext(heapRelation->rd_rel->relowner,
3583+
save_sec_context |SECURITY_RESTRICTED_OPERATION);
3584+
save_nestlevel=NewGUCNestLevel();
3585+
35553586
if (progress)
35563587
{
35573588
constintprogress_cols[]= {
@@ -3770,12 +3801,18 @@ reindex_index(Oid indexId, bool skip_constraint_checks, char persistence,
37703801
errdetail_internal("%s",
37713802
pg_rusage_show(&ru0))));
37723803

3773-
if (progress)
3774-
pgstat_progress_end_command();
3804+
/* Roll back any GUC changes executed by index functions */
3805+
AtEOXact_GUC(false,save_nestlevel);
3806+
3807+
/* Restore userid and security context */
3808+
SetUserIdAndSecContext(save_userid,save_sec_context);
37753809

37763810
/* Close rels, but keep locks */
37773811
index_close(iRel,NoLock);
37783812
table_close(heapRelation,NoLock);
3813+
3814+
if (progress)
3815+
pgstat_progress_end_command();
37793816
}
37803817

37813818
/*

0 commit comments

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