database/postgres
1
0
Fork 0
mirror of https://github.com/postgres/postgres.git synced 2025-07-17 06:41: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 d387588601
commit 7f098f7b53
10 changed files with 382 additions and 50 deletions
Download Patch File Download Diff File Expand all files Collapse all files

98
src/backend/commands/indexcmds.c
View File

@ -470,21 +470,22 @@ DefineIndex(Oid relationId,
LOCKTAG heaplocktag;
LOCKMODE lockmode;
Snapshot snapshot;
int save_nestlevel = -1;
Oid root_save_userid;
int root_save_sec_context;
int root_save_nestlevel;
int i;
root_save_nestlevel = NewGUCNestLevel();
/*
* Some callers need us to run with an empty default_tablespace; this is a
* necessary hack to be able to reproduce catalog state accurately when
* recreating indexes after table-rewriting ALTER TABLE.
*/
if (stmt->reset_default_tblspc)
{
save_nestlevel = NewGUCNestLevel();
(void) set_config_option("default_tablespace", "",
PGC_USERSET, PGC_S_SESSION,
GUC_ACTION_SAVE, true, 0, false);
}
/*
* Force non-concurrent build on temporary relations, even if CONCURRENTLY
@ -563,6 +564,15 @@ DefineIndex(Oid relationId,
lockmode = concurrent ? ShareUpdateExclusiveLock : ShareLock;
rel = table_open(relationId, lockmode);
/*
* Switch to the table owner's userid, so that any index functions are run
* as that user. Also lock down security-restricted operations. We
* already arranged to make GUC variable changes local to this command.
*/
GetUserIdAndSecContext(&root_save_userid, &root_save_sec_context);
SetUserIdAndSecContext(rel->rd_rel->relowner,
root_save_sec_context | SECURITY_RESTRICTED_OPERATION);
namespaceId = RelationGetNamespace(rel);
/* Ensure that it makes sense to index this kind of relation */
@ -648,7 +658,7 @@ DefineIndex(Oid relationId,
{
AclResult aclresult;
aclresult = pg_namespace_aclcheck(namespaceId, GetUserId(),
aclresult = pg_namespace_aclcheck(namespaceId, root_save_userid,
ACL_CREATE);
if (aclresult != ACLCHECK_OK)
aclcheck_error(aclresult, OBJECT_SCHEMA,
@ -680,7 +690,7 @@ DefineIndex(Oid relationId,
{
AclResult aclresult;
aclresult = pg_tablespace_aclcheck(tablespaceId, GetUserId(),
aclresult = pg_tablespace_aclcheck(tablespaceId, root_save_userid,
ACL_CREATE);
if (aclresult != ACLCHECK_OK)
aclcheck_error(aclresult, OBJECT_TABLESPACE,
@ -1066,15 +1076,17 @@ DefineIndex(Oid relationId,
ObjectAddressSet(address, RelationRelationId, indexRelationId);
/*
* Revert to original default_tablespace. Must do this before any return
* from this function, but after index_create, so this is a good time.
*/
if (save_nestlevel >= 0)
AtEOXact_GUC(true, save_nestlevel);
if (!OidIsValid(indexRelationId))
{
/*
* Roll back any GUC changes executed by index functions. Also revert
* to original default_tablespace if we changed it above.
*/
AtEOXact_GUC(false, root_save_nestlevel);
/* Restore userid and security context */
SetUserIdAndSecContext(root_save_userid, root_save_sec_context);
table_close(rel, NoLock);
/* If this is the top-level index, we're done */
@ -1084,6 +1096,17 @@ DefineIndex(Oid relationId,
return address;
}
/*
* Roll back any GUC changes executed by index functions, and keep
* subsequent changes local to this command. It's barely possible that
* some index function changed a behavior-affecting GUC, e.g. xmloption,
* that affects subsequent steps. This improves bug-compatibility with
* older PostgreSQL versions. They did the AtEOXact_GUC() here for the
* purpose of clearing the above default_tablespace change.
*/
AtEOXact_GUC(false, root_save_nestlevel);
root_save_nestlevel = NewGUCNestLevel();
/* Add any requested comment */
if (stmt->idxcomment != NULL)
CreateComments(indexRelationId, RelationRelationId, 0,
@ -1130,6 +1153,9 @@ DefineIndex(Oid relationId,
{
Oid childRelid = part_oids[i];
Relation childrel;
Oid child_save_userid;
int child_save_sec_context;
int child_save_nestlevel;
List *childidxs;
ListCell *cell;
AttrNumber *attmap;
@ -1138,6 +1164,12 @@ DefineIndex(Oid relationId,
childrel = table_open(childRelid, lockmode);
GetUserIdAndSecContext(&child_save_userid,
&child_save_sec_context);
SetUserIdAndSecContext(childrel->rd_rel->relowner,
child_save_sec_context | SECURITY_RESTRICTED_OPERATION);
child_save_nestlevel = NewGUCNestLevel();
/*
* Don't try to create indexes on foreign tables, though. Skip
* those if a regular index, or fail if trying to create a
@ -1153,6 +1185,9 @@ DefineIndex(Oid relationId,
errdetail("Table \"%s\" contains partitions that are foreign tables.",
RelationGetRelationName(rel))));
AtEOXact_GUC(false, child_save_nestlevel);
SetUserIdAndSecContext(child_save_userid,
child_save_sec_context);
table_close(childrel, lockmode);
continue;
}
@ -1226,6 +1261,9 @@ DefineIndex(Oid relationId,
}
list_free(childidxs);
AtEOXact_GUC(false, child_save_nestlevel);
SetUserIdAndSecContext(child_save_userid,
child_save_sec_context);
table_close(childrel, NoLock);
/*
@ -1280,12 +1318,21 @@ DefineIndex(Oid relationId,
if (found_whole_row)
elog(ERROR, "cannot convert whole-row table reference");
/*
* Recurse as the starting user ID. Callee will use that
* for permission checks, then switch again.
*/
Assert(GetUserId() == child_save_userid);
SetUserIdAndSecContext(root_save_userid,
root_save_sec_context);
DefineIndex(childRelid, childStmt,
InvalidOid, /* no predefined OID */
indexRelationId, /* this is our child */
createdConstraintId,
is_alter_table, check_rights, check_not_in_use,
skip_build, quiet);
SetUserIdAndSecContext(child_save_userid,
child_save_sec_context);
}
pgstat_progress_update_param(PROGRESS_CREATEIDX_PARTITIONS_DONE,
@ -1322,12 +1369,17 @@ DefineIndex(Oid relationId,
* Indexes on partitioned tables are not themselves built, so we're
* done here.
*/
AtEOXact_GUC(false, root_save_nestlevel);
SetUserIdAndSecContext(root_save_userid, root_save_sec_context);
table_close(rel, NoLock);
if (!OidIsValid(parentIndexId))
pgstat_progress_end_command();
return address;
}
AtEOXact_GUC(false, root_save_nestlevel);
SetUserIdAndSecContext(root_save_userid, root_save_sec_context);
if (!concurrent)
{
/* Close the heap and we're done, in the non-concurrent case */
@ -3040,6 +3092,9 @@ ReindexRelationConcurrently(Oid relationOid, int options)
Oid newIndexId;
Relation indexRel;
Relation heapRel;
Oid save_userid;
int save_sec_context;
int save_nestlevel;
Relation newIndexRel;
LockRelId *lockrelid;
@ -3047,6 +3102,16 @@ ReindexRelationConcurrently(Oid relationOid, int options)
heapRel = table_open(indexRel->rd_index->indrelid,
ShareUpdateExclusiveLock);
/*
* 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.
*/
GetUserIdAndSecContext(&save_userid, &save_sec_context);
SetUserIdAndSecContext(heapRel->rd_rel->relowner,
save_sec_context | SECURITY_RESTRICTED_OPERATION);
save_nestlevel = NewGUCNestLevel();
/* This function shouldn't be called for temporary relations. */
if (indexRel->rd_rel->relpersistence == RELPERSISTENCE_TEMP)
elog(ERROR, "cannot reindex a temporary table concurrently");
@ -3101,6 +3166,13 @@ ReindexRelationConcurrently(Oid relationOid, int options)
index_close(indexRel, NoLock);
index_close(newIndexRel, NoLock);
/* 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);
table_close(heapRel, NoLock);
}