From ab49ce7c3414ac19e4afb386d7843ce2d2fb8bda 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 7cc08f18cf3..33718aad436 100644 --- a/src/backend/commands/cluster.c +++ b/src/backend/commands/cluster.c @@ -277,6 +277,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); @@ -306,6 +309,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. @@ -317,11 +330,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; } /* @@ -335,8 +347,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)) @@ -347,8 +358,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; } /* @@ -357,8 +367,7 @@ cluster_rel(Oid tableOid, Oid indexOid, ClusterParams *params) if (!get_index_isclustered(indexOid)) { relation_close(OldHeap, AccessExclusiveLock); - pgstat_progress_end_command(); - return; + goto out; } } } @@ -411,8 +420,7 @@ cluster_rel(Oid tableOid, Oid indexOid, ClusterParams *params) !RelationIsPopulated(OldHeap)) { relation_close(OldHeap, AccessExclusiveLock); - pgstat_progress_end_command(); - return; + goto out; } /* @@ -428,6 +436,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