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

Commita117ceb

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 parentf45f8b7 commita117ceb

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
@@ -249,6 +249,9 @@ bt_index_check_internal(Oid indrelid, bool parentcheck, bool heapallindexed,
249249
Relationindrel;
250250
Relationheaprel;
251251
LOCKMODElockmode;
252+
Oidsave_userid;
253+
intsave_sec_context;
254+
intsave_nestlevel;
252255

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

272293
/*
273294
* 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
@@ -1008,6 +1008,9 @@ brin_summarize_range(PG_FUNCTION_ARGS)
10081008
Oidheapoid;
10091009
RelationindexRel;
10101010
RelationheapRel;
1011+
Oidsave_userid;
1012+
intsave_sec_context;
1013+
intsave_nestlevel;
10111014
doublenumSummarized=0;
10121015

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

@@ -1046,7 +1064,7 @@ brin_summarize_range(PG_FUNCTION_ARGS)
10461064
RelationGetRelationName(indexRel))));
10471065

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

@@ -1064,6 +1082,12 @@ brin_summarize_range(PG_FUNCTION_ARGS)
10641082
/* OK, do it */
10651083
brinsummarize(indexRel,heapRel,heapBlk, true,&numSummarized,NULL);
10661084

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

@@ -1102,6 +1126,9 @@ brin_desummarize_range(PG_FUNCTION_ARGS)
11021126
* passed indexoid isn't an index then IndexGetRelation() will fail.
11031127
* Rather than emitting a not-very-helpful error message, postpone
11041128
* complaining, expecting that the is-it-an-index test below will fail.
1129+
*
1130+
* Unlike brin_summarize_range(), autovacuum never calls this. Hence, we
1131+
* don't switch userid.
11051132
*/
11061133
heapoid=IndexGetRelation(indexoid, true);
11071134
if (OidIsValid(heapoid))

‎src/backend/catalog/index.c

Lines changed: 51 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1445,6 +1445,9 @@ index_concurrently_build(Oid heapRelationId,
14451445
OidindexRelationId)
14461446
{
14471447
RelationheapRel;
1448+
Oidsave_userid;
1449+
intsave_sec_context;
1450+
intsave_nestlevel;
14481451
RelationindexRelation;
14491452
IndexInfo*indexInfo;
14501453

@@ -1454,7 +1457,16 @@ index_concurrently_build(Oid heapRelationId,
14541457
/* Open and lock the parent heap relation */
14551458
heapRel=table_open(heapRelationId,ShareUpdateExclusiveLock);
14561459

1457-
/* And the target index relation */
1460+
/*
1461+
* Switch to the table owner's userid, so that any index functions are run
1462+
* as that user. Also lock down security-restricted operations and
1463+
* arrange to make GUC variable changes local to this command.
1464+
*/
1465+
GetUserIdAndSecContext(&save_userid,&save_sec_context);
1466+
SetUserIdAndSecContext(heapRel->rd_rel->relowner,
1467+
save_sec_context |SECURITY_RESTRICTED_OPERATION);
1468+
save_nestlevel=NewGUCNestLevel();
1469+
14581470
indexRelation=index_open(indexRelationId,RowExclusiveLock);
14591471

14601472
/*
@@ -1470,6 +1482,12 @@ index_concurrently_build(Oid heapRelationId,
14701482
/* Now build the index */
14711483
index_build(heapRel,indexRelation,indexInfo, false, true);
14721484

1485+
/* Roll back any GUC changes executed by index functions */
1486+
AtEOXact_GUC(false,save_nestlevel);
1487+
1488+
/* Restore userid and security context */
1489+
SetUserIdAndSecContext(save_userid,save_sec_context);
1490+
14731491
/* Close both the relations, but keep the locks */
14741492
table_close(heapRel,NoLock);
14751493
index_close(indexRelation,NoLock);
@@ -3299,7 +3317,17 @@ validate_index(Oid heapId, Oid indexId, Snapshot snapshot)
32993317

33003318
/* Open and lock the parent heap relation */
33013319
heapRelation=table_open(heapId,ShareUpdateExclusiveLock);
3302-
/* And the target index relation */
3320+
3321+
/*
3322+
* Switch to the table owner's userid, so that any index functions are run
3323+
* as that user. Also lock down security-restricted operations and
3324+
* arrange to make GUC variable changes local to this command.
3325+
*/
3326+
GetUserIdAndSecContext(&save_userid,&save_sec_context);
3327+
SetUserIdAndSecContext(heapRelation->rd_rel->relowner,
3328+
save_sec_context |SECURITY_RESTRICTED_OPERATION);
3329+
save_nestlevel=NewGUCNestLevel();
3330+
33033331
indexRelation=index_open(indexId,RowExclusiveLock);
33043332

33053333
/*
@@ -3312,16 +3340,6 @@ validate_index(Oid heapId, Oid indexId, Snapshot snapshot)
33123340
/* mark build is concurrent just for consistency */
33133341
indexInfo->ii_Concurrent= true;
33143342

3315-
/*
3316-
* Switch to the table owner's userid, so that any index functions are run
3317-
* as that user. Also lock down security-restricted operations and
3318-
* arrange to make GUC variable changes local to this command.
3319-
*/
3320-
GetUserIdAndSecContext(&save_userid,&save_sec_context);
3321-
SetUserIdAndSecContext(heapRelation->rd_rel->relowner,
3322-
save_sec_context |SECURITY_RESTRICTED_OPERATION);
3323-
save_nestlevel=NewGUCNestLevel();
3324-
33253343
/*
33263344
* Scan the index and gather up all the TIDs into a tuplesort object.
33273345
*/
@@ -3530,6 +3548,9 @@ reindex_index(Oid indexId, bool skip_constraint_checks, char persistence,
35303548
RelationiRel,
35313549
heapRelation;
35323550
OidheapId;
3551+
Oidsave_userid;
3552+
intsave_sec_context;
3553+
intsave_nestlevel;
35333554
IndexInfo*indexInfo;
35343555
volatileboolskipped_constraint= false;
35353556
PGRUsageru0;
@@ -3557,6 +3578,16 @@ reindex_index(Oid indexId, bool skip_constraint_checks, char persistence,
35573578
if (!heapRelation)
35583579
return;
35593580

3581+
/*
3582+
* Switch to the table owner's userid, so that any index functions are run
3583+
* as that user. Also lock down security-restricted operations and
3584+
* arrange to make GUC variable changes local to this command.
3585+
*/
3586+
GetUserIdAndSecContext(&save_userid,&save_sec_context);
3587+
SetUserIdAndSecContext(heapRelation->rd_rel->relowner,
3588+
save_sec_context |SECURITY_RESTRICTED_OPERATION);
3589+
save_nestlevel=NewGUCNestLevel();
3590+
35603591
if (progress)
35613592
{
35623593
constintprogress_cols[]= {
@@ -3775,12 +3806,18 @@ reindex_index(Oid indexId, bool skip_constraint_checks, char persistence,
37753806
errdetail_internal("%s",
37763807
pg_rusage_show(&ru0))));
37773808

3778-
if (progress)
3779-
pgstat_progress_end_command();
3809+
/* Roll back any GUC changes executed by index functions */
3810+
AtEOXact_GUC(false,save_nestlevel);
3811+
3812+
/* Restore userid and security context */
3813+
SetUserIdAndSecContext(save_userid,save_sec_context);
37803814

37813815
/* Close rels, but keep locks */
37823816
index_close(iRel,NoLock);
37833817
table_close(heapRelation,NoLock);
3818+
3819+
if (progress)
3820+
pgstat_progress_end_command();
37843821
}
37853822

37863823
/*

0 commit comments

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