From 46670e688fa6c8a0f010c2ab687f4cccaa7c2b31 Mon Sep 17 00:00:00 2001 From: Manish Gupta Date: Wed, 22 Jul 2026 17:18:52 +0530 Subject: [PATCH] [WEB-8332] fix(security): block workspace-member mass-assignment (GHSA-f739-39g5-jj49) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit WorkSpaceMemberViewSet.partial_update passed request.data verbatim into WorkSpaceMemberSerializer (fields="__all__"), making workspace, role, and is_active mass-assignable. A workspace admin could PATCH a member row setting workspace= and role=20, relocating a controlled account into the victim workspace as admin — full cross-tenant takeover. The endpoint's only legitimate mutation is `role`, so restrict the writable payload to {"role": ...}. Note: passing fields=("id","member","role") does NOT work — DynamicBaseSerializer discards the fields= kwarg (base.py) — so the allowlist is enforced in the view instead. Preserves the self-role-update guard and the guest role-cascade. is_active is only changed via destroy(), not here. Adds 4 contract tests; fail-before verified (2 attack tests failed unpatched → 4 pass). Co-authored-by: Plane AI --- apps/api/plane/app/views/workspace/member.py | 13 +- ...st_workspace_member_mass_assignment_app.py | 160 ++++++++++++++++++ 2 files changed, 172 insertions(+), 1 deletion(-) create mode 100644 apps/api/plane/tests/contract/app/test_workspace_member_mass_assignment_app.py diff --git a/apps/api/plane/app/views/workspace/member.py b/apps/api/plane/app/views/workspace/member.py index 67c7637a8c..46eedb3638 100644 --- a/apps/api/plane/app/views/workspace/member.py +++ b/apps/api/plane/app/views/workspace/member.py @@ -88,7 +88,18 @@ class WorkSpaceMemberViewSet(BaseViewSet): if "role" in request.data and int(request.data.get("role")) == 5: ProjectMember.objects.filter(workspace__slug=slug, member_id=workspace_member.member_id).update(role=5) - serializer = WorkSpaceMemberSerializer(workspace_member, data=request.data, partial=True) + # SECURITY: The only field this endpoint is allowed to mutate is ``role``. + # ``WorkSpaceMemberSerializer`` is declared with ``fields = "__all__"`` and + # ``DynamicBaseSerializer`` ignores the ``fields=`` kwarg for writes, so + # passing ``request.data`` verbatim would let a workspace admin mass-assign + # ``workspace`` (relocating a controlled member row into a victim workspace as + # an admin — full cross-tenant takeover), ``is_active``, and other columns. + # Restrict the writable payload to ``role`` only. See GHSA-f739-39g5-jj49. + allowed_data = {} + if "role" in request.data: + allowed_data["role"] = request.data.get("role") + + serializer = WorkSpaceMemberSerializer(workspace_member, data=allowed_data, partial=True) if serializer.is_valid(): serializer.save() diff --git a/apps/api/plane/tests/contract/app/test_workspace_member_mass_assignment_app.py b/apps/api/plane/tests/contract/app/test_workspace_member_mass_assignment_app.py new file mode 100644 index 0000000000..7b7b0acfcb --- /dev/null +++ b/apps/api/plane/tests/contract/app/test_workspace_member_mass_assignment_app.py @@ -0,0 +1,160 @@ +# Copyright (c) 2023-present Plane Software, Inc. and contributors +# SPDX-License-Identifier: AGPL-3.0-only +# See the LICENSE file for details. + +""" +Regression tests for GHSA-f739-39g5-jj49 (WEB-8332). + +``WorkSpaceMemberViewSet.partial_update`` is only meant to update a member's +``role``. Before the fix it passed ``request.data`` verbatim to +``WorkSpaceMemberSerializer`` (declared ``fields = "__all__"`` with only the +nested ``member`` read-only). ``DynamicBaseSerializer`` ignores the ``fields=`` +kwarg for writes, so ``workspace`` (FK), ``is_active`` and other columns were +mass-assignable. + +An admin of an attacker-owned workspace could therefore PATCH a controlled +member row setting ``workspace`` = victim-workspace UUID and ``role`` = 20, +relocating a controlled account into the victim workspace as an admin with no +invitation — a full cross-tenant takeover. The ``@allow_permission(level= +"WORKSPACE")`` decorator authorizes against the URL slug and does NOT prevent +the write from moving the row to a different workspace. + +The fix restricts the writable payload to ``role`` only. +""" + +import uuid + +import pytest +from rest_framework import status +from rest_framework.test import APIClient + +from plane.db.models import User, Workspace, WorkspaceMember + + +def _member_detail_url(slug: str, pk: uuid.UUID) -> str: + return f"/api/workspaces/{slug}/members/{pk}/" + + +def _make_user(email: str) -> User: + local_part = email.split("@")[0] + user = User.objects.create(email=email, username=local_part, first_name=local_part) + user.set_password("test-password") + user.save() + return user + + +def _make_workspace(name: str, slug: str, owner: User) -> Workspace: + workspace = Workspace.objects.create(name=name, slug=slug, owner=owner) + WorkspaceMember.objects.create(workspace=workspace, member=owner, role=20, is_active=True) + return workspace + + +def _add_member(workspace: Workspace, user: User, *, role: int) -> WorkspaceMember: + return WorkspaceMember.objects.create( + workspace=workspace, member=user, role=role, is_active=True + ) + + +@pytest.fixture +def attacker_workspace(db): + """A workspace fully controlled by the attacker (they are the admin/owner).""" + attacker = _make_user("ws-attacker@plane.so") + workspace = _make_workspace("Attacker Workspace", "attacker-ws", attacker) + return workspace, attacker + + +@pytest.fixture +def victim_workspace(db): + """An unrelated workspace the attacker has no membership in.""" + victim_owner = _make_user("ws-victim-owner@plane.so") + workspace = _make_workspace("Victim Workspace", "victim-ws", victim_owner) + return workspace, victim_owner + + +@pytest.mark.contract +@pytest.mark.django_db +class TestWorkspaceMemberMassAssignment: + def test_admin_cannot_move_member_to_another_workspace(self, attacker_workspace, victim_workspace): + """ + Core takeover vector: a workspace admin must not be able to relocate a + member row into another workspace via the ``workspace`` FK. + """ + attacker_ws, attacker = attacker_workspace + victim_ws, _ = victim_workspace + + # A puppet account the attacker controls, sitting in the attacker workspace. + puppet = _make_user("puppet@plane.so") + puppet_member = _add_member(attacker_ws, puppet, role=15) + + victim_members_before = WorkspaceMember.objects.filter(workspace=victim_ws).count() + + client = APIClient() + client.force_authenticate(user=attacker) + response = client.patch( + _member_detail_url(attacker_ws.slug, puppet_member.id), + {"workspace": str(victim_ws.id), "role": 20}, + format="json", + ) + + # The request itself succeeds (admin updating a role is legitimate) ... + assert response.status_code == status.HTTP_200_OK + # ... but the member row must NOT have moved to the victim workspace. + puppet_member.refresh_from_db() + assert puppet_member.workspace_id == attacker_ws.id + assert puppet_member.workspace_id != victim_ws.id + # And no new member appeared in the victim workspace. + assert WorkspaceMember.objects.filter(workspace=victim_ws).count() == victim_members_before + assert not WorkspaceMember.objects.filter(workspace=victim_ws, member=puppet).exists() + + def test_patch_is_active_false_does_not_deactivate(self, attacker_workspace): + """``is_active`` must not be assignable through this endpoint.""" + attacker_ws, attacker = attacker_workspace + target = _make_user("active-target@plane.so") + target_member = _add_member(attacker_ws, target, role=15) + + client = APIClient() + client.force_authenticate(user=attacker) + response = client.patch( + _member_detail_url(attacker_ws.slug, target_member.id), + {"is_active": False}, + format="json", + ) + + assert response.status_code == status.HTTP_200_OK + target_member.refresh_from_db() + assert target_member.is_active is True + + def test_legitimate_role_update_still_works(self, attacker_workspace): + """Positive control: the intended ``role`` update still functions.""" + attacker_ws, attacker = attacker_workspace + target = _make_user("role-target@plane.so") + target_member = _add_member(attacker_ws, target, role=15) + + client = APIClient() + client.force_authenticate(user=attacker) + response = client.patch( + _member_detail_url(attacker_ws.slug, target_member.id), + {"role": 5}, + format="json", + ) + + assert response.status_code == status.HTTP_200_OK + target_member.refresh_from_db() + assert target_member.role == 5 + + def test_self_role_update_still_forbidden(self, attacker_workspace): + """Positive control: the self-role-update guard is preserved.""" + attacker_ws, attacker = attacker_workspace + own_member = WorkspaceMember.objects.get(workspace=attacker_ws, member=attacker) + + client = APIClient() + client.force_authenticate(user=attacker) + response = client.patch( + _member_detail_url(attacker_ws.slug, own_member.id), + {"role": 5}, + format="json", + ) + + assert response.status_code == status.HTTP_400_BAD_REQUEST + own_member.refresh_from_db() + assert own_member.role == 20