From cc3eb974f1142146ddfedf7126cd4dfd376d32d1 Mon Sep 17 00:00:00 2001 From: Manish Gupta <59428681+mguptahub@users.noreply.github.com> Date: Tue, 23 Jun 2026 18:02:52 +0530 Subject: [PATCH] [WEB-7813] fix: prevent ORM order_by injection in issue and other endpoints (#9292) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: prevent ORM order_by injection via user-supplied query params (GHSA-2r95, GHSA-w45q) Add field-name allowlists and a sanitize_order_by() utility in order_queryset.py. All allowlists are centralised there; each call site imports the named constant so there are no inline sets scattered across view files. - order_queryset.py: ISSUE_ORDER_BY_ALLOWLIST, INTAKE_ISSUE_ORDER_BY_ALLOWLIST, ACTIVITY_ORDER_BY_ALLOWLIST, PROJECT_ORDER_BY_ALLOWLIST, VIEW_ORDER_BY_ALLOWLIST, NOTIFICATION_ORDER_BY_ALLOWLIST + sanitize_order_by() utility; validation added at the top of order_issue_queryset() — fixes all callers including the unauthenticated ProjectIssuesPublicEndpoint (GHSA-w45q) - api/views/cycle.py, api/views/module.py: cycle/module issue list endpoints - api/views/issue.py: IssueActivity list and detail endpoints - app/views/intake/base.py: IntakeIssue list - app/views/view/base.py: saved-view list - app/views/notification/base.py: notification paginator - app/views/project/base.py: project list paginator - app/views/user/base.py, app/views/workspace/user.py: activity paginators Closes WEB-7813 Co-authored-by: Plane AI * fix: harden sanitize_order_by against multi-dash malformed inputs lstrip("-") stripped all leading dashes, allowing "--created_at" to pass the allowlist check unchanged and reach .order_by() as a malformed token (causing FieldError). Now strips only one leading dash; any remaining dash prefix is rejected to the safe default. Co-authored-by: Plane AI --------- Co-authored-by: Plane AI --- apps/api/plane/api/views/cycle.py | 3 +- apps/api/plane/api/views/issue.py | 9 +- apps/api/plane/api/views/module.py | 5 +- apps/api/plane/app/views/intake/base.py | 9 +- apps/api/plane/app/views/notification/base.py | 7 +- apps/api/plane/app/views/project/base.py | 7 +- apps/api/plane/app/views/user/base.py | 7 +- apps/api/plane/app/views/view/base.py | 10 +- apps/api/plane/app/views/workspace/user.py | 8 +- apps/api/plane/utils/order_queryset.py | 96 +++++++++++++++++++ 10 files changed, 148 insertions(+), 13 deletions(-) diff --git a/apps/api/plane/api/views/cycle.py b/apps/api/plane/api/views/cycle.py index db58d9d355..99ebb18512 100644 --- a/apps/api/plane/api/views/cycle.py +++ b/apps/api/plane/api/views/cycle.py @@ -46,6 +46,7 @@ from plane.db.models import ( UserFavorite, ) from plane.utils.cycle_transfer_issues import transfer_cycle_issues +from plane.utils.order_queryset import ISSUE_ORDER_BY_ALLOWLIST, sanitize_order_by from plane.utils.host import base_host from .base import BaseAPIView from plane.bgtasks.webhook_task import model_activity @@ -854,7 +855,7 @@ class CycleIssueListCreateAPIEndpoint(BaseAPIView): Returns paginated results with work item details, assignees, and labels. """ # List - order_by = request.GET.get("order_by", "created_at") + order_by = sanitize_order_by(request.GET.get("order_by", "created_at"), ISSUE_ORDER_BY_ALLOWLIST, "created_at") issues = ( Issue.issue_objects.filter(issue_cycle__cycle_id=cycle_id, issue_cycle__deleted_at__isnull=True) .annotate( diff --git a/apps/api/plane/api/views/issue.py b/apps/api/plane/api/views/issue.py index ab439b3ab6..02fcfce6f9 100644 --- a/apps/api/plane/api/views/issue.py +++ b/apps/api/plane/api/views/issue.py @@ -80,6 +80,7 @@ from plane.db.models import ( ) from plane.settings.storage import S3Storage from plane.utils.path_validator import sanitize_filename +from plane.utils.order_queryset import ACTIVITY_ORDER_BY_ALLOWLIST, sanitize_order_by from plane.bgtasks.storage_metadata_task import get_asset_object_metadata from .base import BaseAPIView from plane.utils.host import base_host @@ -1692,7 +1693,9 @@ class IssueActivityListAPIEndpoint(BaseAPIView): ) .filter(project__archived_at__isnull=True) .select_related("actor", "workspace", "issue", "project") - ).order_by(request.GET.get("order_by", "created_at")) + ).order_by( + sanitize_order_by(request.GET.get("order_by", "created_at"), ACTIVITY_ORDER_BY_ALLOWLIST, "created_at") + ) return self.paginate( request=request, @@ -1749,7 +1752,9 @@ class IssueActivityDetailAPIEndpoint(BaseAPIView): .filter(project__archived_at__isnull=True) .select_related("actor", "workspace", "issue", "project") ) - .order_by(request.GET.get("order_by", "created_at")) + .order_by( + sanitize_order_by(request.GET.get("order_by", "created_at"), ACTIVITY_ORDER_BY_ALLOWLIST, "created_at") + ) .first() ) diff --git a/apps/api/plane/api/views/module.py b/apps/api/plane/api/views/module.py index 61e198b481..0a057ebacd 100644 --- a/apps/api/plane/api/views/module.py +++ b/apps/api/plane/api/views/module.py @@ -42,6 +42,7 @@ from plane.db.models import ( from .base import BaseAPIView from plane.bgtasks.webhook_task import model_activity from plane.utils.host import base_host +from plane.utils.order_queryset import ISSUE_ORDER_BY_ALLOWLIST, sanitize_order_by from plane.utils.openapi import ( module_docs, module_issue_docs, @@ -597,7 +598,7 @@ class ModuleIssueListCreateAPIEndpoint(BaseAPIView): Retrieve all work items assigned to a module with detailed information. Returns paginated results including assignees, labels, and attachments. """ - order_by = request.GET.get("order_by", "created_at") + order_by = sanitize_order_by(request.GET.get("order_by", "created_at"), ISSUE_ORDER_BY_ALLOWLIST, "created_at") issues = ( Issue.issue_objects.filter(issue_module__module_id=module_id, issue_module__deleted_at__isnull=True) .annotate( @@ -803,7 +804,7 @@ class ModuleIssueDetailAPIEndpoint(BaseAPIView): Retrieve all work items assigned to a module with detailed information. Returns paginated results including assignees, labels, and attachments. """ - order_by = request.GET.get("order_by", "created_at") + order_by = sanitize_order_by(request.GET.get("order_by", "created_at"), ISSUE_ORDER_BY_ALLOWLIST, "created_at") issues = ( Issue.issue_objects.filter( issue_module__module_id=module_id, diff --git a/apps/api/plane/app/views/intake/base.py b/apps/api/plane/app/views/intake/base.py index d4049aa3cb..e51376e663 100644 --- a/apps/api/plane/app/views/intake/base.py +++ b/apps/api/plane/app/views/intake/base.py @@ -44,6 +44,7 @@ from plane.app.serializers import ( IssueDescriptionVersionDetailSerializer, ) from plane.utils.issue_filters import issue_filters +from plane.utils.order_queryset import INTAKE_ISSUE_ORDER_BY_ALLOWLIST, sanitize_order_by from plane.bgtasks.issue_activities_task import issue_activity from plane.bgtasks.issue_description_version_task import issue_description_version_task from plane.app.views.base import BaseAPIView @@ -195,7 +196,13 @@ class IntakeIssueViewSet(BaseViewSet): Value([], output_field=ArrayField(UUIDField())), ) ) - ).order_by(request.GET.get("order_by", "-issue__created_at")) + ).order_by( + sanitize_order_by( + request.GET.get("order_by", "-issue__created_at"), + INTAKE_ISSUE_ORDER_BY_ALLOWLIST, + "-issue__created_at", + ) + ) # Intake status filter intake_status = [item for item in request.GET.get("status", "-2").split(",") if item != "null"] if intake_status: diff --git a/apps/api/plane/app/views/notification/base.py b/apps/api/plane/app/views/notification/base.py index 0b7dc27a8a..5f8d6efae6 100644 --- a/apps/api/plane/app/views/notification/base.py +++ b/apps/api/plane/app/views/notification/base.py @@ -23,6 +23,7 @@ from plane.db.models import ( WorkspaceMember, ) from plane.utils.paginator import BasePaginator +from plane.utils.order_queryset import NOTIFICATION_ORDER_BY_ALLOWLIST, sanitize_order_by from plane.app.permissions import allow_permission, ROLE # Module imports @@ -139,7 +140,11 @@ class NotificationViewSet(BaseViewSet, BasePaginator): # Pagination if request.GET.get("per_page", False) and request.GET.get("cursor", False): return self.paginate( - order_by=request.GET.get("order_by", "-created_at"), + order_by=sanitize_order_by( + request.GET.get("order_by", "-created_at"), + NOTIFICATION_ORDER_BY_ALLOWLIST, + "-created_at", + ), request=request, queryset=(notifications), on_results=lambda notifications: NotificationSerializer(notifications, many=True).data, diff --git a/apps/api/plane/app/views/project/base.py b/apps/api/plane/app/views/project/base.py index 57489943fe..ec55dcb9df 100644 --- a/apps/api/plane/app/views/project/base.py +++ b/apps/api/plane/app/views/project/base.py @@ -41,6 +41,7 @@ from plane.db.models import ( ) from plane.db.models.intake import IntakeIssueStatus from plane.utils.host import base_host +from plane.utils.order_queryset import PROJECT_ORDER_BY_ALLOWLIST, sanitize_order_by class ProjectViewSet(BaseViewSet): @@ -128,7 +129,11 @@ class ProjectViewSet(BaseViewSet): if request.GET.get("per_page", False) and request.GET.get("cursor", False): return self.paginate( - order_by=request.GET.get("order_by", "-created_at"), + order_by=sanitize_order_by( + request.GET.get("order_by", "-created_at"), + PROJECT_ORDER_BY_ALLOWLIST, + "-created_at", + ), request=request, queryset=(projects), on_results=lambda projects: ProjectListSerializer(projects, many=True).data, diff --git a/apps/api/plane/app/views/user/base.py b/apps/api/plane/app/views/user/base.py index 914dffb3b0..5956792d98 100644 --- a/apps/api/plane/app/views/user/base.py +++ b/apps/api/plane/app/views/user/base.py @@ -45,6 +45,7 @@ from plane.db.models import ( ) from plane.license.models import Instance, InstanceAdmin from plane.utils.paginator import BasePaginator +from plane.utils.order_queryset import ACTIVITY_ORDER_BY_ALLOWLIST, sanitize_order_by from plane.authentication.utils.host import user_ip from plane.bgtasks.user_deactivation_email_task import user_deactivation_email from plane.utils.host import base_host @@ -384,7 +385,11 @@ class UserActivityEndpoint(BaseAPIView, BasePaginator): ) return self.paginate( - order_by=request.GET.get("order_by", "-created_at"), + order_by=sanitize_order_by( + request.GET.get("order_by", "-created_at"), + ACTIVITY_ORDER_BY_ALLOWLIST, + "-created_at", + ), request=request, queryset=queryset, on_results=lambda issue_activities: IssueActivitySerializer(issue_activities, many=True).data, diff --git a/apps/api/plane/app/views/view/base.py b/apps/api/plane/app/views/view/base.py index 5ca7aac420..3aad55abcc 100644 --- a/apps/api/plane/app/views/view/base.py +++ b/apps/api/plane/app/views/view/base.py @@ -41,7 +41,7 @@ from plane.db.models import ( ModuleIssue, ) from plane.utils.issue_filters import issue_filters -from plane.utils.order_queryset import order_issue_queryset +from plane.utils.order_queryset import VIEW_ORDER_BY_ALLOWLIST, order_issue_queryset, sanitize_order_by from plane.bgtasks.recent_visited_task import recent_visited_task from .. import BaseViewSet from plane.db.models import UserFavorite @@ -64,7 +64,13 @@ class WorkspaceViewViewSet(BaseViewSet): .filter(workspace__slug=self.kwargs.get("slug")) .filter(project__isnull=True) .filter(Q(owned_by=self.request.user) | Q(access=1)) - .order_by(self.request.GET.get("order_by", "-created_at")) + .order_by( + sanitize_order_by( + self.request.GET.get("order_by", "-created_at"), + VIEW_ORDER_BY_ALLOWLIST, + "-created_at", + ) + ) .distinct() ) diff --git a/apps/api/plane/app/views/workspace/user.py b/apps/api/plane/app/views/workspace/user.py index 5b028a32ff..2d5f171e31 100644 --- a/apps/api/plane/app/views/workspace/user.py +++ b/apps/api/plane/app/views/workspace/user.py @@ -59,7 +59,7 @@ from plane.utils.grouper import ( issue_queryset_grouper, ) from plane.utils.issue_filters import issue_filters -from plane.utils.order_queryset import order_issue_queryset +from plane.utils.order_queryset import ACTIVITY_ORDER_BY_ALLOWLIST, order_issue_queryset, sanitize_order_by from plane.utils.paginator import GroupedOffsetPaginator, SubGroupedOffsetPaginator from plane.utils.filters import ComplexFilterBackend from plane.utils.filters import IssueFilterSet @@ -391,7 +391,11 @@ class WorkspaceUserActivityEndpoint(BaseAPIView): queryset = queryset.filter(project__in=projects) return self.paginate( - order_by=request.GET.get("order_by", "-created_at"), + order_by=sanitize_order_by( + request.GET.get("order_by", "-created_at"), + ACTIVITY_ORDER_BY_ALLOWLIST, + "-created_at", + ), request=request, queryset=queryset, on_results=lambda issue_activities: IssueActivitySerializer(issue_activities, many=True).data, diff --git a/apps/api/plane/utils/order_queryset.py b/apps/api/plane/utils/order_queryset.py index abc0bbca0c..1ef2b05689 100644 --- a/apps/api/plane/utils/order_queryset.py +++ b/apps/api/plane/utils/order_queryset.py @@ -8,8 +8,104 @@ from django.db.models import Case, CharField, Min, Value, When PRIORITY_ORDER = ["urgent", "high", "medium", "low", "none"] STATE_ORDER = ["backlog", "unstarted", "started", "completed", "cancelled"] +# --------------------------------------------------------------------------- +# order_by allowlists — one per model/endpoint family +# All contain bare field names (no leading '-'); the sanitizer strips the +# prefix before looking up, so descending variants are implicitly covered. +# Prevents ORM order_by injection via user-supplied query params +# (GHSA-2r95-c453-vxmr / GHSA-w45q-6m65-9498). +# --------------------------------------------------------------------------- + +ISSUE_ORDER_BY_ALLOWLIST = frozenset({ + "created_at", + "updated_at", + "sequence_id", + "sort_order", + "target_date", + "start_date", + "completed_at", + "archived_at", + "priority", + "state__name", + "state__group", + "assignees__first_name", + "labels__name", + "issue_module__module__name", +}) + +# IntakeIssue queryset — fields are prefixed with `issue__` for the join. +INTAKE_ISSUE_ORDER_BY_ALLOWLIST = frozenset({ + "issue__created_at", + "issue__updated_at", + "issue__sequence_id", + "issue__sort_order", + "issue__target_date", + "issue__start_date", + "issue__priority", + "issue__state__name", + "created_at", + "updated_at", + "status", +}) + +# IssueActivity queryset (user activity, workspace member activity). +ACTIVITY_ORDER_BY_ALLOWLIST = frozenset({ + "created_at", + "updated_at", +}) + +# Project list queryset. +PROJECT_ORDER_BY_ALLOWLIST = frozenset({ + "created_at", + "updated_at", + "name", + "network", + "sort_order", +}) + +# Saved view (IssueView) list queryset. +VIEW_ORDER_BY_ALLOWLIST = frozenset({ + "created_at", + "updated_at", + "name", +}) + +# Notification queryset. +NOTIFICATION_ORDER_BY_ALLOWLIST = frozenset({ + "created_at", + "updated_at", +}) + + +def sanitize_order_by(value, allowed_fields, default="-created_at"): + """Return a safe ordering string derived from *value*. + + Strips at most one leading '-' (descending indicator), checks the bare + field name against *allowed_fields*, and reconstructs the value. Inputs + with multiple leading dashes (e.g. ``--created_at``) are rejected and + *default* is returned instead, preventing both allowlist bypass and + malformed tokens from reaching ``.order_by()``. + + Call this before passing any user-supplied ordering string to .order_by() or + a paginator to prevent ORM order_by injection. + """ + if not value: + return default + is_desc = value.startswith("-") + bare = value[1:] if is_desc else value + # Reject malformed prefixes like "--created_at". + if bare.startswith("-"): + return default + if bare not in allowed_fields: + return default + return f"-{bare}" if is_desc else bare + def order_issue_queryset(issue_queryset, order_by_param="-created_at"): + # Reject any field that is not in the allowlist before building the queryset. + # An unrecognised value is silently replaced with the safe default so callers + # receive consistent output rather than an ORM error or data leak. + order_by_param = sanitize_order_by(order_by_param, ISSUE_ORDER_BY_ALLOWLIST, default="-created_at") # Priority Ordering if order_by_param == "priority" or order_by_param == "-priority": issue_queryset = issue_queryset.annotate(