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

Commit7f098f7

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 parentd387588 commit7f098f7

File tree

10 files changed

+382
-50
lines changed

10 files changed

+382
-50
lines changed

‎contrib/amcheck/expected/check_btree.out

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

169169
(1 row)
170170

171+
--
172+
-- Check that index expressions and predicates are run as the table's owner
173+
--
174+
TRUNCATE bttest_a;
175+
INSERT INTO bttest_a SELECT * FROM generate_series(1, 1000);
176+
ALTER TABLE bttest_a OWNER TO regress_bttest_role;
177+
-- A dummy index function checking current_user
178+
CREATE FUNCTION ifun(int8) RETURNS int8 AS $$
179+
BEGIN
180+
ASSERT current_user = 'regress_bttest_role',
181+
format('ifun(%s) called by %s', $1, current_user);
182+
RETURN $1;
183+
END;
184+
$$ LANGUAGE plpgsql IMMUTABLE;
185+
CREATE INDEX bttest_a_expr_idx ON bttest_a ((ifun(id) + ifun(0)))
186+
WHERE ifun(id + 10) > ifun(10);
187+
SELECT bt_index_check('bttest_a_expr_idx', true);
188+
bt_index_check
189+
----------------
190+
191+
(1 row)
192+
171193
-- cleanup
172194
DROP TABLE bttest_a;
173195
DROP TABLE bttest_b;
174196
DROP TABLE bttest_multi;
175197
DROP TABLE delete_test_table;
176198
DROP TABLE toast_bug;
199+
DROP FUNCTION ifun(int8);
177200
DROP OWNED BY regress_bttest_role; -- permissions
178201
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
@@ -110,11 +110,32 @@ INSERT INTO toast_bug SELECT repeat('a', 2200);
110110
-- Should not get false positive report of corruption:
111111
SELECT bt_index_check('toasty', true);
112112

113+
--
114+
-- Check that index expressions and predicates are run as the table's owner
115+
--
116+
TRUNCATE bttest_a;
117+
INSERT INTO bttest_aSELECT*FROM generate_series(1,1000);
118+
ALTERTABLE bttest_a OWNER TO regress_bttest_role;
119+
-- A dummy index function checking current_user
120+
CREATEFUNCTIONifun(int8) RETURNS int8AS $$
121+
BEGIN
122+
ASSERTcurrent_user='regress_bttest_role',
123+
format('ifun(%s) called by %s', $1,current_user);
124+
RETURN $1;
125+
END;
126+
$$ LANGUAGE plpgsql IMMUTABLE;
127+
128+
CREATEINDEXbttest_a_expr_idxON bttest_a ((ifun(id)+ ifun(0)))
129+
WHERE ifun(id+10)> ifun(10);
130+
131+
SELECT bt_index_check('bttest_a_expr_idx', true);
132+
113133
-- cleanup
114134
DROPTABLE bttest_a;
115135
DROPTABLE bttest_b;
116136
DROPTABLE bttest_multi;
117137
DROPTABLE delete_test_table;
118138
DROPTABLE toast_bug;
139+
DROPFUNCTION ifun(int8);
119140
DROP OWNED BY regress_bttest_role;-- permissions
120141
DROP ROLE regress_bttest_role;

‎contrib/amcheck/verify_nbtree.c

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -228,6 +228,9 @@ bt_index_check_internal(Oid indrelid, bool parentcheck, bool heapallindexed,
228228
Relationindrel;
229229
Relationheaprel;
230230
LOCKMODElockmode;
231+
Oidsave_userid;
232+
intsave_sec_context;
233+
intsave_nestlevel;
231234

232235
if (parentcheck)
233236
lockmode=ShareLock;
@@ -244,9 +247,27 @@ bt_index_check_internal(Oid indrelid, bool parentcheck, bool heapallindexed,
244247
*/
245248
heapid=IndexGetRelation(indrelid, true);
246249
if (OidIsValid(heapid))
250+
{
247251
heaprel=table_open(heapid,lockmode);
252+
253+
/*
254+
* Switch to the table owner's userid, so that any index functions are
255+
* run as that user. Also lock down security-restricted operations
256+
* and arrange to make GUC variable changes local to this command.
257+
*/
258+
GetUserIdAndSecContext(&save_userid,&save_sec_context);
259+
SetUserIdAndSecContext(heaprel->rd_rel->relowner,
260+
save_sec_context |SECURITY_RESTRICTED_OPERATION);
261+
save_nestlevel=NewGUCNestLevel();
262+
}
248263
else
264+
{
249265
heaprel=NULL;
266+
/* for "gcc -Og" https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78394 */
267+
save_userid=InvalidOid;
268+
save_sec_context=-1;
269+
save_nestlevel=-1;
270+
}
250271

251272
/*
252273
* Open the target index relations separately (like relation_openrv(), but
@@ -293,6 +314,12 @@ bt_index_check_internal(Oid indrelid, bool parentcheck, bool heapallindexed,
293314
heapallindexed,rootdescend);
294315
}
295316

317+
/* Roll back any GUC changes executed by index functions */
318+
AtEOXact_GUC(false,save_nestlevel);
319+
320+
/* Restore userid and security context */
321+
SetUserIdAndSecContext(save_userid,save_sec_context);
322+
296323
/*
297324
* Release locks early. That's ok here because nothing in the called
298325
* 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
@@ -873,6 +873,9 @@ brin_summarize_range(PG_FUNCTION_ARGS)
873873
Oidheapoid;
874874
RelationindexRel;
875875
RelationheapRel;
876+
Oidsave_userid;
877+
intsave_sec_context;
878+
intsave_nestlevel;
876879
doublenumSummarized=0;
877880

878881
if (RecoveryInProgress())
@@ -899,7 +902,22 @@ brin_summarize_range(PG_FUNCTION_ARGS)
899902
*/
900903
heapoid=IndexGetRelation(indexoid, true);
901904
if (OidIsValid(heapoid))
905+
{
902906
heapRel=table_open(heapoid,ShareUpdateExclusiveLock);
907+
908+
/*
909+
* Autovacuum calls us. For its benefit, switch to the table owner's
910+
* userid, so that any index functions are run as that user. Also
911+
* lock down security-restricted operations and arrange to make GUC
912+
* variable changes local to this command. This is harmless, albeit
913+
* unnecessary, when called from SQL, because we fail shortly if the
914+
* user does not own the index.
915+
*/
916+
GetUserIdAndSecContext(&save_userid,&save_sec_context);
917+
SetUserIdAndSecContext(heapRel->rd_rel->relowner,
918+
save_sec_context |SECURITY_RESTRICTED_OPERATION);
919+
save_nestlevel=NewGUCNestLevel();
920+
}
903921
else
904922
heapRel=NULL;
905923

@@ -914,7 +932,7 @@ brin_summarize_range(PG_FUNCTION_ARGS)
914932
RelationGetRelationName(indexRel))));
915933

916934
/* User must own the index (comparable to privileges needed for VACUUM) */
917-
if (!pg_class_ownercheck(indexoid,GetUserId()))
935+
if (heapRel!=NULL&& !pg_class_ownercheck(indexoid,save_userid))
918936
aclcheck_error(ACLCHECK_NOT_OWNER,OBJECT_INDEX,
919937
RelationGetRelationName(indexRel));
920938

@@ -932,6 +950,12 @@ brin_summarize_range(PG_FUNCTION_ARGS)
932950
/* OK, do it */
933951
brinsummarize(indexRel,heapRel,heapBlk, true,&numSummarized,NULL);
934952

953+
/* Roll back any GUC changes executed by index functions */
954+
AtEOXact_GUC(false,save_nestlevel);
955+
956+
/* Restore userid and security context */
957+
SetUserIdAndSecContext(save_userid,save_sec_context);
958+
935959
relation_close(indexRel,ShareUpdateExclusiveLock);
936960
relation_close(heapRel,ShareUpdateExclusiveLock);
937961

@@ -973,6 +997,9 @@ brin_desummarize_range(PG_FUNCTION_ARGS)
973997
* passed indexoid isn't an index then IndexGetRelation() will fail.
974998
* Rather than emitting a not-very-helpful error message, postpone
975999
* complaining, expecting that the is-it-an-index test below will fail.
1000+
*
1001+
* Unlike brin_summarize_range(), autovacuum never calls this. Hence, we
1002+
* don't switch userid.
9761003
*/
9771004
heapoid=IndexGetRelation(indexoid, true);
9781005
if (OidIsValid(heapoid))

‎src/backend/catalog/index.c

Lines changed: 51 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1400,6 +1400,9 @@ index_concurrently_build(Oid heapRelationId,
14001400
OidindexRelationId)
14011401
{
14021402
RelationheapRel;
1403+
Oidsave_userid;
1404+
intsave_sec_context;
1405+
intsave_nestlevel;
14031406
RelationindexRelation;
14041407
IndexInfo*indexInfo;
14051408

@@ -1409,7 +1412,16 @@ index_concurrently_build(Oid heapRelationId,
14091412
/* Open and lock the parent heap relation */
14101413
heapRel=table_open(heapRelationId,ShareUpdateExclusiveLock);
14111414

1412-
/* And the target index relation */
1415+
/*
1416+
* Switch to the table owner's userid, so that any index functions are run
1417+
* as that user. Also lock down security-restricted operations and
1418+
* arrange to make GUC variable changes local to this command.
1419+
*/
1420+
GetUserIdAndSecContext(&save_userid,&save_sec_context);
1421+
SetUserIdAndSecContext(heapRel->rd_rel->relowner,
1422+
save_sec_context |SECURITY_RESTRICTED_OPERATION);
1423+
save_nestlevel=NewGUCNestLevel();
1424+
14131425
indexRelation=index_open(indexRelationId,RowExclusiveLock);
14141426

14151427
/*
@@ -1425,6 +1437,12 @@ index_concurrently_build(Oid heapRelationId,
14251437
/* Now build the index */
14261438
index_build(heapRel,indexRelation,indexInfo, false, true);
14271439

1440+
/* Roll back any GUC changes executed by index functions */
1441+
AtEOXact_GUC(false,save_nestlevel);
1442+
1443+
/* Restore userid and security context */
1444+
SetUserIdAndSecContext(save_userid,save_sec_context);
1445+
14281446
/* Close both the relations, but keep the locks */
14291447
table_close(heapRel,NoLock);
14301448
index_close(indexRelation,NoLock);
@@ -3282,7 +3300,17 @@ validate_index(Oid heapId, Oid indexId, Snapshot snapshot)
32823300

32833301
/* Open and lock the parent heap relation */
32843302
heapRelation=table_open(heapId,ShareUpdateExclusiveLock);
3285-
/* And the target index relation */
3303+
3304+
/*
3305+
* Switch to the table owner's userid, so that any index functions are run
3306+
* as that user. Also lock down security-restricted operations and
3307+
* arrange to make GUC variable changes local to this command.
3308+
*/
3309+
GetUserIdAndSecContext(&save_userid,&save_sec_context);
3310+
SetUserIdAndSecContext(heapRelation->rd_rel->relowner,
3311+
save_sec_context |SECURITY_RESTRICTED_OPERATION);
3312+
save_nestlevel=NewGUCNestLevel();
3313+
32863314
indexRelation=index_open(indexId,RowExclusiveLock);
32873315

32883316
/*
@@ -3295,16 +3323,6 @@ validate_index(Oid heapId, Oid indexId, Snapshot snapshot)
32953323
/* mark build is concurrent just for consistency */
32963324
indexInfo->ii_Concurrent= true;
32973325

3298-
/*
3299-
* Switch to the table owner's userid, so that any index functions are run
3300-
* as that user. Also lock down security-restricted operations and
3301-
* arrange to make GUC variable changes local to this command.
3302-
*/
3303-
GetUserIdAndSecContext(&save_userid,&save_sec_context);
3304-
SetUserIdAndSecContext(heapRelation->rd_rel->relowner,
3305-
save_sec_context |SECURITY_RESTRICTED_OPERATION);
3306-
save_nestlevel=NewGUCNestLevel();
3307-
33083326
/*
33093327
* Scan the index and gather up all the TIDs into a tuplesort object.
33103328
*/
@@ -3508,6 +3526,9 @@ reindex_index(Oid indexId, bool skip_constraint_checks, char persistence,
35083526
RelationiRel,
35093527
heapRelation;
35103528
OidheapId;
3529+
Oidsave_userid;
3530+
intsave_sec_context;
3531+
intsave_nestlevel;
35113532
IndexInfo*indexInfo;
35123533
volatileboolskipped_constraint= false;
35133534
PGRUsageru0;
@@ -3522,6 +3543,16 @@ reindex_index(Oid indexId, bool skip_constraint_checks, char persistence,
35223543
heapId=IndexGetRelation(indexId, false);
35233544
heapRelation=table_open(heapId,ShareLock);
35243545

3546+
/*
3547+
* Switch to the table owner's userid, so that any index functions are run
3548+
* as that user. Also lock down security-restricted operations and
3549+
* arrange to make GUC variable changes local to this command.
3550+
*/
3551+
GetUserIdAndSecContext(&save_userid,&save_sec_context);
3552+
SetUserIdAndSecContext(heapRelation->rd_rel->relowner,
3553+
save_sec_context |SECURITY_RESTRICTED_OPERATION);
3554+
save_nestlevel=NewGUCNestLevel();
3555+
35253556
if (progress)
35263557
{
35273558
pgstat_progress_start_command(PROGRESS_COMMAND_CREATE_INDEX,
@@ -3695,12 +3726,18 @@ reindex_index(Oid indexId, bool skip_constraint_checks, char persistence,
36953726
errdetail_internal("%s",
36963727
pg_rusage_show(&ru0))));
36973728

3698-
if (progress)
3699-
pgstat_progress_end_command();
3729+
/* Roll back any GUC changes executed by index functions */
3730+
AtEOXact_GUC(false,save_nestlevel);
3731+
3732+
/* Restore userid and security context */
3733+
SetUserIdAndSecContext(save_userid,save_sec_context);
37003734

37013735
/* Close rels, but keep locks */
37023736
index_close(iRel,NoLock);
37033737
table_close(heapRelation,NoLock);
3738+
3739+
if (progress)
3740+
pgstat_progress_end_command();
37043741
}
37053742

37063743
/*

0 commit comments

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