-
Notifications
You must be signed in to change notification settings - Fork 396
Description
Summary
The UpdateTenantDependentModelMutation and DeleteTenantDependentModelMutation base classes accept a tenant_id parameter but never use it to scope database queries, allowing cross-tenant data modification/deletion (CWE-639).
Vulnerability Details
Root Cause: packages/backend/common/graphql/mutations.py, line 366
@classmethod
def get_queryset(cls, model_class, root, info, **input):
return model_class.objects.all() # Returns ALL objects from ALL tenantsUpdateTenantDependentModelMutation (lines 654-673) converts tenant_id from global ID to local ID but never passes it to the queryset:
class UpdateTenantDependentModelMutation(UpdateModelMutation):
@classmethod
def mutate_and_get_payload(cls, root, info, **input):
if "tenant_id" in input:
_, input["tenant_id"] = from_global_id(input["tenant_id"])
return super().mutate_and_get_payload(root, info, **input)The parent's get_object() calls get_queryset() which returns model_class.objects.all() — finds ANY object by ID regardless of tenant.
Secure pattern comparison — Query resolvers in apps/demo/schema.py correctly scope by tenant:
@permission_classes(IsTenantMemberAccess)
def resolve_crud_demo_item(root, info, id, tenant_id, **kwargs):
_, pk = from_global_id(id)
_, tenant_pk = from_global_id(tenant_id)
return get_object_or_404(models.CrudDemoItem, pk=pk, tenant=tenant_pk) # Scoped!The @permission_classes(IsTenantMemberAccess) on TenantMemberMutation validates the user is a member of the claimed tenant, but since get_queryset() returns ALL objects, a user can pass their OWN tenant_id (passes permission check) with a target resource's id from another tenant (not scoped).
Impact
As a SaaS boilerplate, this pattern is likely copied into production applications. Affects UpdateCrudDemoItemMutation, DeleteCrudDemoItemMutation, and any app using these base classes.
Recommended Fix
Override get_queryset() in tenant-dependent base classes to filter by tenant_id.
Disclosure
Found during security research. Happy to provide additional details.