diff --git a/apps/api/plane/app/views/page/base.py b/apps/api/plane/app/views/page/base.py index ec391afc1aa..b33466c6af8 100644 --- a/apps/api/plane/app/views/page/base.py +++ b/apps/api/plane/app/views/page/base.py @@ -46,6 +46,7 @@ UserRecentVisit, ) from plane.utils.error_codes import ERROR_CODES +from plane.utils.order_queryset import PAGE_ORDER_BY_ALLOWLIST, sanitize_order_by # Local imports from ..base import BaseAPIView, BaseViewSet @@ -100,9 +101,23 @@ def get_queryset(self): .select_related("workspace") .select_related("owned_by") .annotate(is_favorite=Exists(subquery)) - .order_by(self.request.GET.get("order_by", "-created_at")) .prefetch_related("labels") - .order_by("-is_favorite", "-created_at") + # Sanitize the user-supplied order_by against an allowlist: Django + # resolves the field at call time, so an unknown field raises + # FieldError (500 DoS) and a relation path (e.g. owned_by__password) + # enables ORM relational traversal (GHSA-2v48). Favourites stay + # pinned first; the sanitized user ordering is the secondary sort + # (a single .order_by() so it is not overridden), with id as a + # stable tiebreak for pagination. + .order_by( + "-is_favorite", + sanitize_order_by( + self.request.GET.get("order_by", "-created_at"), + PAGE_ORDER_BY_ALLOWLIST, + default="-created_at", + ), + "id", + ) .annotate( project=Exists( ProjectPage.objects.filter(page_id=OuterRef("id"), project_id=self.kwargs.get("project_id")) diff --git a/apps/api/plane/tests/contract/app/test_page_order_by_allowlist_app.py b/apps/api/plane/tests/contract/app/test_page_order_by_allowlist_app.py new file mode 100644 index 00000000000..16a049be99f --- /dev/null +++ b/apps/api/plane/tests/contract/app/test_page_order_by_allowlist_app.py @@ -0,0 +1,93 @@ +# 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-2v48-qcjw-74ch (page order_by ORM injection). + +PageViewSet.get_queryset passed the raw order_by query param into .order_by(), +which resolves field names at call time — an unknown field raised FieldError +(500 DoS) and a relation path (e.g. owned_by__password) enabled ORM relational +traversal. The param is now sanitized against PAGE_ORDER_BY_ALLOWLIST. +""" + +import pytest +from rest_framework import status + +from plane.db.models import Page, Project, ProjectMember, ProjectPage + + +@pytest.fixture +def project_with_page(db, workspace, create_user): + project = Project.objects.create(name="P", identifier="PRD", workspace=workspace) + ProjectMember.objects.create( + workspace=workspace, project=project, member=create_user, role=20, is_active=True + ) + page = Page.objects.create(workspace=workspace, owned_by=create_user, access=Page.PUBLIC_ACCESS, name="pg") + ProjectPage.objects.create(workspace=workspace, project=project, page=page) + return project, page + + +def _pages_url(slug, project_id): + return f"/api/workspaces/{slug}/projects/{project_id}/pages/" + + +@pytest.mark.contract +class TestPageOrderByAllowlist: + @pytest.mark.django_db + @pytest.mark.parametrize( + "order_by", + [ + "password", # invalid field → FieldError (500) pre-fix + "bogus__field__x", # invalid relation path → FieldError (500) pre-fix + "owned_by__password", # valid relation path → ORM traversal pre-fix + ], + ) + def test_malicious_order_by_is_rejected(self, session_client, workspace, project_with_page, order_by): + project, _ = project_with_page + + response = session_client.get(_pages_url(workspace.slug, project.id), {"order_by": order_by}) + + # Sanitized to the safe default — no 500, no traversal. + assert response.status_code == status.HTTP_200_OK + + @pytest.mark.django_db + @pytest.mark.parametrize("order_by", ["name", "-name", "created_at", "-created_at", "updated_at", "sort_order"]) + def test_allowlisted_order_by_is_accepted(self, session_client, workspace, project_with_page, order_by): + project, _ = project_with_page + + response = session_client.get(_pages_url(workspace.slug, project.id), {"order_by": order_by}) + + assert response.status_code == status.HTTP_200_OK + + @pytest.mark.django_db + def test_no_order_by_param_defaults_ok(self, session_client, workspace, project_with_page): + project, _ = project_with_page + + response = session_client.get(_pages_url(workspace.slug, project.id)) + + assert response.status_code == status.HTTP_200_OK + + @pytest.mark.django_db + def test_allowlisted_order_by_actually_orders_results(self, session_client, workspace, create_user): + """An allowlisted order_by must actually affect the result ordering — + guards against the param being silently overridden by a later + .order_by() call.""" + project = Project.objects.create(name="P2", identifier="ORD", workspace=workspace) + ProjectMember.objects.create( + workspace=workspace, project=project, member=create_user, role=20, is_active=True + ) + # Non-favorite public pages so the favourite-first primary sort is a + # no-op and the secondary (name) ordering is observable. + for name in ("Gamma", "Alpha", "Beta"): + page = Page.objects.create( + workspace=workspace, owned_by=create_user, access=Page.PUBLIC_ACCESS, name=name + ) + ProjectPage.objects.create(workspace=workspace, project=project, page=page) + + asc = session_client.get(_pages_url(workspace.slug, project.id), {"order_by": "name"}) + desc = session_client.get(_pages_url(workspace.slug, project.id), {"order_by": "-name"}) + + assert asc.status_code == status.HTTP_200_OK + assert [p["name"] for p in asc.json()] == ["Alpha", "Beta", "Gamma"] + assert [p["name"] for p in desc.json()] == ["Gamma", "Beta", "Alpha"] diff --git a/apps/api/plane/utils/order_queryset.py b/apps/api/plane/utils/order_queryset.py index c799dadbd70..ba9ca17cb91 100644 --- a/apps/api/plane/utils/order_queryset.py +++ b/apps/api/plane/utils/order_queryset.py @@ -76,6 +76,14 @@ "updated_at", }) +# Page list queryset. +PAGE_ORDER_BY_ALLOWLIST = frozenset({ + "created_at", + "updated_at", + "name", + "sort_order", +}) + # --------------------------------------------------------------------------- # group_by / sub_group_by allowlist for Issue querysets — used by # GroupedOffsetPaginator / SubGroupedOffsetPaginator (plane/utils/paginator.py), @@ -118,6 +126,7 @@ "sort_order", }) + def sanitize_order_by(value, allowed_fields, default="-created_at"): """Return a safe ordering string derived from *value*.