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

Commit35edcc0

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 parent9164637 commit35edcc0

File tree

10 files changed

+381
-48
lines changed

10 files changed

+381
-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
@@ -245,6 +245,9 @@ bt_index_check_internal(Oid indrelid, bool parentcheck, bool heapallindexed,
245245
Relationindrel;
246246
Relationheaprel;
247247
LOCKMODElockmode;
248+
Oidsave_userid;
249+
intsave_sec_context;
250+
intsave_nestlevel;
248251

249252
if (parentcheck)
250253
lockmode=ShareLock;
@@ -261,9 +264,27 @@ bt_index_check_internal(Oid indrelid, bool parentcheck, bool heapallindexed,
261264
*/
262265
heapid=IndexGetRelation(indrelid, true);
263266
if (OidIsValid(heapid))
267+
{
264268
heaprel=table_open(heapid,lockmode);
269+
270+
/*
271+
* Switch to the table owner's userid, so that any index functions are
272+
* run as that user. Also lock down security-restricted operations
273+
* and arrange to make GUC variable changes local to this command.
274+
*/
275+
GetUserIdAndSecContext(&save_userid,&save_sec_context);
276+
SetUserIdAndSecContext(heaprel->rd_rel->relowner,
277+
save_sec_context |SECURITY_RESTRICTED_OPERATION);
278+
save_nestlevel=NewGUCNestLevel();
279+
}
265280
else
281+
{
266282
heaprel=NULL;
283+
/* for "gcc -Og" https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78394 */
284+
save_userid=InvalidOid;
285+
save_sec_context=-1;
286+
save_nestlevel=-1;
287+
}
267288

268289
/*
269290
* Open the target index relations separately (like relation_openrv(), but
@@ -323,6 +344,12 @@ bt_index_check_internal(Oid indrelid, bool parentcheck, bool heapallindexed,
323344
heapallindexed,rootdescend);
324345
}
325346

347+
/* Roll back any GUC changes executed by index functions */
348+
AtEOXact_GUC(false,save_nestlevel);
349+
350+
/* Restore userid and security context */
351+
SetUserIdAndSecContext(save_userid,save_sec_context);
352+
326353
/*
327354
* Release locks early. That's ok here because nothing in the called
328355
* 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
@@ -865,6 +865,9 @@ brin_summarize_range(PG_FUNCTION_ARGS)
865865
Oidheapoid;
866866
RelationindexRel;
867867
RelationheapRel;
868+
Oidsave_userid;
869+
intsave_sec_context;
870+
intsave_nestlevel;
868871
doublenumSummarized=0;
869872

870873
if (RecoveryInProgress())
@@ -891,7 +894,22 @@ brin_summarize_range(PG_FUNCTION_ARGS)
891894
*/
892895
heapoid=IndexGetRelation(indexoid, true);
893896
if (OidIsValid(heapoid))
897+
{
894898
heapRel=table_open(heapoid,ShareUpdateExclusiveLock);
899+
900+
/*
901+
* Autovacuum calls us. For its benefit, switch to the table owner's
902+
* userid, so that any index functions are run as that user. Also
903+
* lock down security-restricted operations and arrange to make GUC
904+
* variable changes local to this command. This is harmless, albeit
905+
* unnecessary, when called from SQL, because we fail shortly if the
906+
* user does not own the index.
907+
*/
908+
GetUserIdAndSecContext(&save_userid,&save_sec_context);
909+
SetUserIdAndSecContext(heapRel->rd_rel->relowner,
910+
save_sec_context |SECURITY_RESTRICTED_OPERATION);
911+
save_nestlevel=NewGUCNestLevel();
912+
}
895913
else
896914
heapRel=NULL;
897915

@@ -906,7 +924,7 @@ brin_summarize_range(PG_FUNCTION_ARGS)
906924
RelationGetRelationName(indexRel))));
907925

908926
/* User must own the index (comparable to privileges needed for VACUUM) */
909-
if (!pg_class_ownercheck(indexoid,GetUserId()))
927+
if (heapRel!=NULL&& !pg_class_ownercheck(indexoid,save_userid))
910928
aclcheck_error(ACLCHECK_NOT_OWNER,OBJECT_INDEX,
911929
RelationGetRelationName(indexRel));
912930

@@ -924,6 +942,12 @@ brin_summarize_range(PG_FUNCTION_ARGS)
924942
/* OK, do it */
925943
brinsummarize(indexRel,heapRel,heapBlk, true,&numSummarized,NULL);
926944

945+
/* Roll back any GUC changes executed by index functions */
946+
AtEOXact_GUC(false,save_nestlevel);
947+
948+
/* Restore userid and security context */
949+
SetUserIdAndSecContext(save_userid,save_sec_context);
950+
927951
relation_close(indexRel,ShareUpdateExclusiveLock);
928952
relation_close(heapRel,ShareUpdateExclusiveLock);
929953

@@ -965,6 +989,9 @@ brin_desummarize_range(PG_FUNCTION_ARGS)
965989
* passed indexoid isn't an index then IndexGetRelation() will fail.
966990
* Rather than emitting a not-very-helpful error message, postpone
967991
* complaining, expecting that the is-it-an-index test below will fail.
992+
*
993+
* Unlike brin_summarize_range(), autovacuum never calls this. Hence, we
994+
* don't switch userid.
968995
*/
969996
heapoid=IndexGetRelation(indexoid, true);
970997
if (OidIsValid(heapoid))

‎src/backend/catalog/index.c

Lines changed: 51 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1421,6 +1421,9 @@ index_concurrently_build(Oid heapRelationId,
14211421
OidindexRelationId)
14221422
{
14231423
RelationheapRel;
1424+
Oidsave_userid;
1425+
intsave_sec_context;
1426+
intsave_nestlevel;
14241427
RelationindexRelation;
14251428
IndexInfo*indexInfo;
14261429

@@ -1430,7 +1433,16 @@ index_concurrently_build(Oid heapRelationId,
14301433
/* Open and lock the parent heap relation */
14311434
heapRel=table_open(heapRelationId,ShareUpdateExclusiveLock);
14321435

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

14361448
/*
@@ -1446,6 +1458,12 @@ index_concurrently_build(Oid heapRelationId,
14461458
/* Now build the index */
14471459
index_build(heapRel,indexRelation,indexInfo, false, true);
14481460

1461+
/* Roll back any GUC changes executed by index functions */
1462+
AtEOXact_GUC(false,save_nestlevel);
1463+
1464+
/* Restore userid and security context */
1465+
SetUserIdAndSecContext(save_userid,save_sec_context);
1466+
14491467
/* Close both the relations, but keep the locks */
14501468
table_close(heapRel,NoLock);
14511469
index_close(indexRelation,NoLock);
@@ -3283,7 +3301,17 @@ validate_index(Oid heapId, Oid indexId, Snapshot snapshot)
32833301

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

32893317
/*
@@ -3296,16 +3324,6 @@ validate_index(Oid heapId, Oid indexId, Snapshot snapshot)
32963324
/* mark build is concurrent just for consistency */
32973325
indexInfo->ii_Concurrent= true;
32983326

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

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

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

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

37073744
/*

0 commit comments

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