From e467062c945be39f88b657c7db44f770a53489fe Mon Sep 17 00:00:00 2001
From: tom27052006 <83423411+tom27052006@users.noreply.github.com>
Date: Thu, 23 Jul 2026 18:37:03 +0200
Subject: [PATCH] Fix privilege escalation: collection manage grants bypassed
 delete gate

A per-collection `manage` row carries collection delete authority via
`has_explicit_collection_manage_access` -> `CollectionDeleteHeaders`, so
only a caller who could delete the collection may confer it. Two write
paths still passed the client-supplied `manage` bit through unguarded:

- `send_invite` gated the initial collection assignments only on
  `has_full_access()`, which `edit_any_collection` alone satisfies, so a
  Custom member with manage-users and edit-any-collection could plant a
  manage row on any collection for an account they control.

- `post_organization_collections` wrote `group.manage` / `user.manage`
  directly. With the create guard narrowed to `can_create_new_collections()`,
  a create-only Custom member could grant manage on the new collection to
  itself, another member, or a group.

Both now AND the requested bit with `caller_may_grant_collection_manage`,
the same gate `post_bulk_access_collections`,
`post_organization_collection_update`, `edit_member` and `add_update_group`
already use. In the create handler the gate is evaluated once, before both
assignment loops, so no grant can bootstrap the next. The check is strictly
subtractive: Admin/Owner, Custom-with-delete-any-collection and the
access_all Manager are unaffected.
---
 src/api/core/organizations.rs | 38 ++++++++++++++++++++++++++++++-----
 1 file changed, 33 insertions(+), 5 deletions(-)

diff --git a/src/api/core/organizations.rs b/src/api/core/organizations.rs
index 718bf21a..edc88881 100644
--- a/src/api/core/organizations.rs
+++ b/src/api/core/organizations.rs
@@ -576,10 +576,27 @@ async fn post_organization_collections(
     )
     .await;
 
+    // Security (F-3): a `manage` grant carries collection *delete*/administer authority
+    // (`has_explicit_collection_manage_access` -> CollectionDeleteHeaders/ManagerHeaders), so only a
+    // caller who could delete this collection may confer it — the same rule the collection-update and
+    // bulk-access endpoints apply. Create is deliberately independent from Edit/Delete, so a Custom
+    // member holding only `create_new_collections` must not be able to hand a manage row to another
+    // member or to a group (nor to itself) while creating the collection. For such callers the
+    // requested `manage` is forced to false; Admin/Owner, Custom-with-`delete_any_collection` and the
+    // legacy access_all Manager keep it. Evaluated after the collection exists so the per-collection
+    // lookup sees it.
+    let may_grant_manage = caller_may_grant_collection_manage(&headers.membership, &collection.uuid, &conn).await;
+
     for group in data.groups {
-        CollectionGroup::new(collection.uuid.clone(), group.id, group.read_only, group.hide_passwords, group.manage)
-            .save(&org_id, &conn)
-            .await?;
+        CollectionGroup::new(
+            collection.uuid.clone(),
+            group.id,
+            group.read_only,
+            group.hide_passwords,
+            group.manage && may_grant_manage,
+        )
+        .save(&org_id, &conn)
+        .await?;
     }
 
     for user in data.users {
@@ -596,7 +613,7 @@ async fn post_organization_collections(
             &collection.uuid,
             user.read_only,
             user.hide_passwords,
-            user.manage,
+            user.manage && may_grant_manage,
             &conn,
         )
         .await?;
@@ -1371,16 +1388,27 @@ async fn send_invite(
 
         // If no accessAll, add the collections received
         if !access_all && caller_can_manage_collections {
+            // Security (F-1): a per-collection `manage` grant carries delete authority, so the
+            // caller may only confer it on collections they could delete themselves. Otherwise a
+            // caller acting via Edit-any-collection could invite an account they control with a
+            // `manage` row and reach Delete-any-collection through it.
+            let caller = Membership::find_by_user_and_org(&headers.user.uuid, &org_id, &conn).await;
+
             for col in data.collections.iter().flatten() {
                 match Collection::find_by_uuid_and_org(&col.id, &org_id, &conn).await {
                     None => err!("Collection not found in Organization"),
                     Some(collection) => {
+                        let manage = col.manage
+                            && match &caller {
+                                Some(c) => caller_may_grant_collection_manage(c, &collection.uuid, &conn).await,
+                                None => false,
+                            };
                         CollectionUser::save(
                             &user.uuid,
                             &collection.uuid,
                             col.read_only,
                             col.hide_passwords,
-                            col.manage,
+                            manage,
                             &conn,
                         )
                         .await?;