mirror of
https://github.com/makeplane/plane.git
synced 2026-07-09 12:00:38 +02:00
[WEB-7813] fix: prevent ORM order_by injection in issue and other endpoints (#9292)
* 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 <noreply@plane.so> * 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 <noreply@plane.so> --------- Co-authored-by: Plane AI <noreply@plane.so>
This commit is contained in:
@@ -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(
|
||||
|
||||
@@ -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()
|
||||
)
|
||||
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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:
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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()
|
||||
)
|
||||
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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(
|
||||
|
||||
Reference in New Issue
Block a user