From 9f45aa77e7998411dc45f831226a22d42ac859a3 Mon Sep 17 00:00:00 2001 From: tom27052006 <83423411+tom27052006@users.noreply.github.com> Date: Tue, 7 Jul 2026 14:12:36 +0200 Subject: [PATCH] Fix privilege escalation: gate access_all in edit_member A Custom member with only manage_users could set the "manage all collections" child permissions (createNewCollections / editAnyCollection / deleteAnyCollection) on any member, including themselves, via POST /organizations//users/. edit_member wrote member_to_edit.access_all unconditionally, so the resulting access_all=true granted full access to every collection's contents - defeating the "manage users without collection access" guarantee. Gate the access_all change on caller_can_manage_collections (Admins/Owners or full-access members), mirroring how put_group preserves a group's access_all for callers without collection rights. For everyone else the member's existing access_all is left untouched. The collection- and group-assignment paths were already gated; this closes the remaining direct path. --- src/api/core/organizations.rs | 27 +++++++++++++++++---------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/src/api/core/organizations.rs b/src/api/core/organizations.rs index ba8ab681..e8403fc1 100644 --- a/src/api/core/organizations.rs +++ b/src/api/core/organizations.rs @@ -1642,16 +1642,6 @@ async fn edit_member( err!("Only Admins or Owners can grant custom management permissions") } - member_to_edit.access_all = access_all; - member_to_edit.manage_users = manage_users; - member_to_edit.manage_groups = manage_groups; - member_to_edit.manage_policies = manage_policies; - member_to_edit.atype = new_type as i32; - - // This check is also done at accept_invite, _confirm_invite, _activate_member, edit_member, admin::update_membership_type - // We need to perform the check after changing the type since `admin` is exempt. - OrgPolicy::check_user_allowed(&member_to_edit, "modify", &conn).await?; - // Security: only callers who can actually manage collections (Admins/Owners, or users // with full access) may change a member's collection assignments. A custom user with only // manage_users must not be able to add/remove collection access, so we leave the existing @@ -1662,6 +1652,23 @@ async fn edit_member( None => false, }; + // Security: `access_all` grants full access to every collection, so only callers who may + // manage collections are allowed to change it. Otherwise a custom user with only manage_users + // could set the Custom "manage all collections" child boxes on any member (including + // themselves) to grant full collection access — a privilege escalation. For everyone else we + // keep the member's existing access_all grant untouched (neither granted nor revoked). + if caller_can_manage_collections { + member_to_edit.access_all = access_all; + } + member_to_edit.manage_users = manage_users; + member_to_edit.manage_groups = manage_groups; + member_to_edit.manage_policies = manage_policies; + member_to_edit.atype = new_type as i32; + + // This check is also done at accept_invite, _confirm_invite, _activate_member, edit_member, admin::update_membership_type + // We need to perform the check after changing the type since `admin` is exempt. + OrgPolicy::check_user_allowed(&member_to_edit, "modify", &conn).await?; + if caller_can_manage_collections { // Delete all the odd collections for c in CollectionUser::find_by_organization_and_user_uuid(&org_id, &member_to_edit.user_uuid, &conn).await {