From a117cebd638dd02e5c2e791c25e43745f233111b Mon Sep 17 00:00:00 2001 From: Noah Misch Date: Mon, 9 May 2022 08:35:08 -0700 Subject: Make relation-enumerating operations be security-restricted operations. 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 --- src/backend/commands/cluster.c | 37 ++++++++++++++++++++++++++----------- 1 file changed, 26 insertions(+), 11 deletions(-) (limited to 'src/backend/commands/cluster.c') diff --git a/src/backend/commands/cluster.c b/src/backend/commands/cluster.c index d8a6d43d959..cea2c8be805 100644 --- a/src/backend/commands/cluster.c +++ b/src/backend/commands/cluster.c @@ -310,6 +310,9 @@ void cluster_rel(Oid tableOid, Oid indexOid, ClusterParams *params) { Relation OldHeap; + Oid save_userid; + int save_sec_context; + int save_nestlevel; bool verbose = ((params->options & CLUOPT_VERBOSE) != 0); bool recheck = ((params->options & CLUOPT_RECHECK) != 0); @@ -339,6 +342,16 @@ cluster_rel(Oid tableOid, Oid indexOid, ClusterParams *params) return; } + /* + * 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(OldHeap->rd_rel->relowner, + save_sec_context | SECURITY_RESTRICTED_OPERATION); + save_nestlevel = NewGUCNestLevel(); + /* * Since we may open a new transaction for each relation, we have to check * that the relation still is what we think it is. @@ -350,11 +363,10 @@ cluster_rel(Oid tableOid, Oid indexOid, ClusterParams *params) if (recheck) { /* Check that the user still owns the relation */ - if (!pg_class_ownercheck(tableOid, GetUserId())) + if (!pg_class_ownercheck(tableOid, save_userid)) { relation_close(OldHeap, AccessExclusiveLock); - pgstat_progress_end_command(); - return; + goto out; } /* @@ -369,8 +381,7 @@ cluster_rel(Oid tableOid, Oid indexOid, ClusterParams *params) if (RELATION_IS_OTHER_TEMP(OldHeap)) { relation_close(OldHeap, AccessExclusiveLock); - pgstat_progress_end_command(); - return; + goto out; } if (OidIsValid(indexOid)) @@ -381,8 +392,7 @@ cluster_rel(Oid tableOid, Oid indexOid, ClusterParams *params) if (!SearchSysCacheExists1(RELOID, ObjectIdGetDatum(indexOid))) { relation_close(OldHeap, AccessExclusiveLock); - pgstat_progress_end_command(); - return; + goto out; } /* @@ -393,8 +403,7 @@ cluster_rel(Oid tableOid, Oid indexOid, ClusterParams *params) !get_index_isclustered(indexOid)) { relation_close(OldHeap, AccessExclusiveLock); - pgstat_progress_end_command(); - return; + goto out; } } } @@ -447,8 +456,7 @@ cluster_rel(Oid tableOid, Oid indexOid, ClusterParams *params) !RelationIsPopulated(OldHeap)) { relation_close(OldHeap, AccessExclusiveLock); - pgstat_progress_end_command(); - return; + goto out; } Assert(OldHeap->rd_rel->relkind == RELKIND_RELATION || @@ -468,6 +476,13 @@ cluster_rel(Oid tableOid, Oid indexOid, ClusterParams *params) /* NB: rebuild_relation does table_close() on OldHeap */ +out: + /* 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); + pgstat_progress_end_command(); } -- cgit v1.2.3