database/postgres
1
0
Fork 0
mirror of https://github.com/postgres/postgres.git synced 2025-11-09 06:21:09 +03:00
Code Activity

Make relation-enumerating operations be security-restricted operations.

Browse Source 
When a feature enumerates relations and runs functions associated with
all found relations, the feature's user shall not need to trust every
user having permission to create objects.  BRIN-specific functionality
in autovacuum neglected to account for this, as did pg_amcheck and
CLUSTER.  An attacker having permission to create non-temp objects in at
least one schema could execute arbitrary SQL functions under the
identity of the bootstrap superuser.  CREATE INDEX (not a
relation-enumerating operation) and REINDEX protected themselves too
late.  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
This commit is contained in:
Noah Misch
2022-05-09 08:35:08 -07:00
parent f45f8b7ff3
commit a117cebd63
10 changed files with 378 additions and 48 deletions
Download Patch File Download Diff File Expand all files Collapse all files

29
src/backend/access/brin/brin.c
View File

@@ -1008,6 +1008,9 @@ brin_summarize_range(PG_FUNCTION_ARGS)
Oid heapoid;
Relation indexRel;
Relation heapRel;
Oid save_userid;
int save_sec_context;
int save_nestlevel;
double numSummarized = 0;
if (RecoveryInProgress())
@@ -1031,7 +1034,22 @@ brin_summarize_range(PG_FUNCTION_ARGS)
*/
heapoid = IndexGetRelation(indexoid, true);
if (OidIsValid(heapoid))
{
heapRel = table_open(heapoid, ShareUpdateExclusiveLock);
/*
* Autovacuum calls us. For its benefit, switch to the table owner's
* userid, so that any index functions are run as that user. Also
* lock down security-restricted operations and arrange to make GUC
* variable changes local to this command. This is harmless, albeit
* unnecessary, when called from SQL, because we fail shortly if the
* user does not own the index.
*/
GetUserIdAndSecContext(&save_userid, &save_sec_context);
SetUserIdAndSecContext(heapRel->rd_rel->relowner,
save_sec_context | SECURITY_RESTRICTED_OPERATION);
save_nestlevel = NewGUCNestLevel();
}
else
heapRel = NULL;
@@ -1046,7 +1064,7 @@ brin_summarize_range(PG_FUNCTION_ARGS)
RelationGetRelationName(indexRel))));
/* User must own the index (comparable to privileges needed for VACUUM) */
if (!pg_class_ownercheck(indexoid, GetUserId()))
if (heapRel != NULL && !pg_class_ownercheck(indexoid, save_userid))
aclcheck_error(ACLCHECK_NOT_OWNER, OBJECT_INDEX,
RelationGetRelationName(indexRel));
@@ -1064,6 +1082,12 @@ brin_summarize_range(PG_FUNCTION_ARGS)
/* OK, do it */
brinsummarize(indexRel, heapRel, heapBlk, true, &numSummarized, NULL);
/* Roll back any GUC changes executed by index functions */
AtEOXact_GUC(false, save_nestlevel);
/* Restore userid and security context */
SetUserIdAndSecContext(save_userid, save_sec_context);
relation_close(indexRel, ShareUpdateExclusiveLock);
relation_close(heapRel, ShareUpdateExclusiveLock);
@@ -1102,6 +1126,9 @@ brin_desummarize_range(PG_FUNCTION_ARGS)
* passed indexoid isn't an index then IndexGetRelation() will fail.
* Rather than emitting a not-very-helpful error message, postpone
* complaining, expecting that the is-it-an-index test below will fail.
*
* Unlike brin_summarize_range(), autovacuum never calls this. Hence, we
* don't switch userid.
*/
heapoid = IndexGetRelation(indexoid, true);
if (OidIsValid(heapoid))