Browse Source

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.
pull/7397/head
tom27052006 3 weeks ago
parent
commit
e467062c94
  1. 34
      src/api/core/organizations.rs

34
src/api/core/organizations.rs

@ -576,8 +576,25 @@ 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)
CollectionGroup::new(
collection.uuid.clone(),
group.id,
group.read_only,
group.hide_passwords,
group.manage && may_grant_manage,
)
.save(&org_id, &conn)
.await?;
}
@ -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?;

Loading…
Cancel
Save