From aea66f53f4022bd51cbc829bc03baa048c03e011 Mon Sep 17 00:00:00 2001 From: sriram veeraghanta Date: Mon, 20 Apr 2026 15:33:30 +0530 Subject: [PATCH] fix: sanitize filenames in upload paths to prevent path traversal (#8879) * fix: sanitize filenames in upload paths to prevent path traversal (GHSA-v57h-5999-w7xp) Add server-side filename sanitization across all file upload endpoints to prevent path traversal sequences (../) in user-supplied filenames from being incorporated into S3 object keys. While S3 keys are flat strings and not vulnerable to filesystem traversal, this adds defense-in-depth and prevents S3 key pollution. Changes: - Add sanitize_filename() utility in path_validator.py - Sanitize filenames in get_upload_path() for FileAsset and IssueAttachment models - Sanitize name parameter in all upload view endpoints * fix: address PR review feedback on filename sanitization - Remove unused `import re` - Normalize backslashes to forward slashes before os.path.basename() so Windows-style paths (e.g. ..\..\..\evil.txt) are handled on POSIX - Strip whitespace before removing leading dots so " .env" is caught - Return None instead of "unnamed" for empty input so existing `if not name` validation guards remain effective - Add `or "unnamed"` fallback at call sites that lack a name guard * fix: use random hex name as fallback in get_upload_path instead of "unnamed" * fix: resolve ruff E501 line too long in DuplicateAssetEndpoint --- apps/api/plane/api/views/asset.py | 7 ++-- apps/api/plane/api/views/issue.py | 3 +- apps/api/plane/app/views/asset/v2.py | 10 +++-- apps/api/plane/app/views/issue/attachment.py | 3 +- apps/api/plane/db/models/asset.py | 3 ++ apps/api/plane/db/models/issue.py | 2 + apps/api/plane/space/views/asset.py | 3 +- apps/api/plane/utils/path_validator.py | 41 ++++++++++++++++++++ 8 files changed, 62 insertions(+), 10 deletions(-) diff --git a/apps/api/plane/api/views/asset.py b/apps/api/plane/api/views/asset.py index 88c34c37ca..72ce60e681 100644 --- a/apps/api/plane/api/views/asset.py +++ b/apps/api/plane/api/views/asset.py @@ -17,6 +17,7 @@ from drf_spectacular.utils import OpenApiExample, OpenApiRequest # Module Imports from plane.bgtasks.storage_metadata_task import get_asset_object_metadata from plane.settings.storage import S3Storage +from plane.utils.path_validator import sanitize_filename from plane.db.models import FileAsset, User, Workspace from plane.api.views.base import BaseAPIView from plane.api.serializers import ( @@ -114,7 +115,7 @@ class UserAssetEndpoint(BaseAPIView): This endpoint generates the necessary credentials for direct S3 upload. """ # get the asset key - name = request.data.get("name") + name = sanitize_filename(request.data.get("name")) or "unnamed" type = request.data.get("type", "image/jpeg") size = int(request.data.get("size", settings.FILE_SIZE_LIMIT)) entity_type = request.data.get("entity_type", False) @@ -287,7 +288,7 @@ class UserServerAssetEndpoint(BaseAPIView): necessary credentials for direct S3 upload with server-side authentication. """ # get the asset key - name = request.data.get("name") + name = sanitize_filename(request.data.get("name")) or "unnamed" type = request.data.get("type", "image/jpeg") size = int(request.data.get("size", settings.FILE_SIZE_LIMIT)) entity_type = request.data.get("entity_type", False) @@ -498,7 +499,7 @@ class GenericAssetEndpoint(BaseAPIView): Create a presigned URL for uploading generic assets that can be bound to entities like work items. Supports various file types and includes external source tracking for integrations. """ - name = request.data.get("name") + name = sanitize_filename(request.data.get("name")) type = request.data.get("type") size = int(request.data.get("size", settings.FILE_SIZE_LIMIT)) project_id = request.data.get("project_id") diff --git a/apps/api/plane/api/views/issue.py b/apps/api/plane/api/views/issue.py index 180d684475..b48be56d41 100644 --- a/apps/api/plane/api/views/issue.py +++ b/apps/api/plane/api/views/issue.py @@ -79,6 +79,7 @@ from plane.db.models import ( Workspace, ) from plane.settings.storage import S3Storage +from plane.utils.path_validator import sanitize_filename from plane.bgtasks.storage_metadata_task import get_asset_object_metadata from .base import BaseAPIView from plane.utils.host import base_host @@ -1858,7 +1859,7 @@ class IssueAttachmentListCreateAPIEndpoint(BaseAPIView): status=status.HTTP_403_FORBIDDEN, ) - name = request.data.get("name") + name = sanitize_filename(request.data.get("name")) type = request.data.get("type", False) size = request.data.get("size") external_id = request.data.get("external_id") diff --git a/apps/api/plane/app/views/asset/v2.py b/apps/api/plane/app/views/asset/v2.py index a83c5ec998..b21f70d61f 100644 --- a/apps/api/plane/app/views/asset/v2.py +++ b/apps/api/plane/app/views/asset/v2.py @@ -22,6 +22,7 @@ from plane.db.models import FileAsset, Workspace, Project, User, WorkspaceMember from plane.settings.storage import S3Storage from plane.app.permissions import allow_permission, ROLE from plane.utils.cache import invalidate_cache_directly +from plane.utils.path_validator import sanitize_filename from plane.bgtasks.storage_metadata_task import get_asset_object_metadata from plane.throttles.asset import AssetRateThrottle @@ -108,7 +109,7 @@ class UserAssetsV2Endpoint(BaseAPIView): def post(self, request): # get the asset key - name = request.data.get("name") + name = sanitize_filename(request.data.get("name")) or "unnamed" type = request.data.get("type", "image/jpeg") size = int(request.data.get("size", settings.FILE_SIZE_LIMIT)) entity_type = request.data.get("entity_type", False) @@ -313,7 +314,7 @@ class WorkspaceFileAssetEndpoint(BaseAPIView): @allow_permission([ROLE.ADMIN, ROLE.MEMBER, ROLE.GUEST], level="WORKSPACE") def post(self, request, slug): - name = request.data.get("name") + name = sanitize_filename(request.data.get("name")) or "unnamed" type = request.data.get("type", "image/jpeg") size = int(request.data.get("size", settings.FILE_SIZE_LIMIT)) entity_type = request.data.get("entity_type") @@ -515,7 +516,7 @@ class ProjectAssetEndpoint(BaseAPIView): @allow_permission([ROLE.ADMIN, ROLE.MEMBER, ROLE.GUEST]) def post(self, request, slug, project_id): - name = request.data.get("name") + name = sanitize_filename(request.data.get("name")) or "unnamed" type = request.data.get("type", "image/jpeg") size = int(request.data.get("size", settings.FILE_SIZE_LIMIT)) entity_type = request.data.get("entity_type", "") @@ -770,7 +771,8 @@ class DuplicateAssetEndpoint(BaseAPIView): if not original_asset: return Response({"error": "Asset not found"}, status=status.HTTP_404_NOT_FOUND) - destination_key = f"{workspace.id}/{uuid.uuid4().hex}-{original_asset.attributes.get('name')}" + sanitized_name = sanitize_filename(original_asset.attributes.get("name")) or "unnamed" + destination_key = f"{workspace.id}/{uuid.uuid4().hex}-{sanitized_name}" duplicated_asset = FileAsset.objects.create( attributes={ "name": original_asset.attributes.get("name"), diff --git a/apps/api/plane/app/views/issue/attachment.py b/apps/api/plane/app/views/issue/attachment.py index df027c413b..51248b8a42 100644 --- a/apps/api/plane/app/views/issue/attachment.py +++ b/apps/api/plane/app/views/issue/attachment.py @@ -24,6 +24,7 @@ from plane.db.models import FileAsset, Workspace from plane.bgtasks.issue_activities_task import issue_activity from plane.app.permissions import allow_permission, ROLE from plane.settings.storage import S3Storage +from plane.utils.path_validator import sanitize_filename from plane.bgtasks.storage_metadata_task import get_asset_object_metadata from plane.utils.host import base_host @@ -97,7 +98,7 @@ class IssueAttachmentV2Endpoint(BaseAPIView): @allow_permission([ROLE.ADMIN, ROLE.MEMBER, ROLE.GUEST]) def post(self, request, slug, project_id, issue_id): - name = request.data.get("name") + name = sanitize_filename(request.data.get("name")) or "unnamed" type = request.data.get("type", False) size = int(request.data.get("size", settings.FILE_SIZE_LIMIT)) diff --git a/apps/api/plane/db/models/asset.py b/apps/api/plane/db/models/asset.py index d309135bca..55efff7f41 100644 --- a/apps/api/plane/db/models/asset.py +++ b/apps/api/plane/db/models/asset.py @@ -11,10 +11,13 @@ from django.core.exceptions import ValidationError from django.db import models # Module import +from plane.utils.path_validator import sanitize_filename + from .base import BaseModel def get_upload_path(instance, filename): + filename = sanitize_filename(filename) or uuid4().hex if instance.workspace_id is not None: return f"{instance.workspace.id}/{uuid4().hex}-{filename}" return f"user-{uuid4().hex}-{filename}" diff --git a/apps/api/plane/db/models/issue.py b/apps/api/plane/db/models/issue.py index d24efc8a23..f4175c4785 100644 --- a/apps/api/plane/db/models/issue.py +++ b/apps/api/plane/db/models/issue.py @@ -17,6 +17,7 @@ from django import apps # Module imports from plane.utils.html_processor import strip_tags +from plane.utils.path_validator import sanitize_filename from plane.db.mixins import SoftDeletionManager from plane.utils.exception_logger import log_exception from .project import ProjectBaseModel @@ -376,6 +377,7 @@ class IssueLink(ProjectBaseModel): def get_upload_path(instance, filename): + filename = sanitize_filename(filename) or uuid4().hex return f"{instance.workspace.id}/{uuid4().hex}-{filename}" diff --git a/apps/api/plane/space/views/asset.py b/apps/api/plane/space/views/asset.py index 1749a8fd46..bc20724ca8 100644 --- a/apps/api/plane/space/views/asset.py +++ b/apps/api/plane/space/views/asset.py @@ -18,6 +18,7 @@ from rest_framework.response import Response from plane.bgtasks.storage_metadata_task import get_asset_object_metadata from plane.db.models import DeployBoard, FileAsset from plane.settings.storage import S3Storage +from plane.utils.path_validator import sanitize_filename # Module imports from .base import BaseAPIView @@ -73,7 +74,7 @@ class EntityAssetEndpoint(BaseAPIView): return Response({"error": "Project is not published"}, status=status.HTTP_404_NOT_FOUND) # Get the asset - name = request.data.get("name") + name = sanitize_filename(request.data.get("name")) or "unnamed" type = request.data.get("type", "image/jpeg") size = int(request.data.get("size", settings.FILE_SIZE_LIMIT)) entity_type = request.data.get("entity_type", "") diff --git a/apps/api/plane/utils/path_validator.py b/apps/api/plane/utils/path_validator.py index f15fb4ca95..901464ee6a 100644 --- a/apps/api/plane/utils/path_validator.py +++ b/apps/api/plane/utils/path_validator.py @@ -7,9 +7,50 @@ from django.utils.http import url_has_allowed_host_and_scheme from django.conf import settings # Python imports +import os from urllib.parse import urlparse +def sanitize_filename(filename): + """ + Sanitize a filename to prevent path traversal attacks. + + Strips directory components, path traversal sequences, and null bytes + from user-supplied filenames used in upload paths and S3 object keys. + + Returns None for empty/missing input so callers can still validate + that a filename was provided. + """ + if not filename or not isinstance(filename, str): + return None + + # Strip null bytes + filename = filename.replace("\x00", "") + + # Normalize backslashes so os.path.basename handles Windows-style paths on POSIX + filename = filename.replace("\\", "/") + + # Take only the basename to remove any directory components + filename = os.path.basename(filename) + + # Remove any remaining path traversal sequences + filename = filename.replace("..", "") + + # Strip whitespace before removing leading dots so " .env" is caught + filename = filename.strip() + + # Remove leading dots (hidden files) + filename = filename.lstrip(".") + + # Strip any remaining whitespace + filename = filename.strip() + + if not filename: + return None + + return filename + + def _contains_suspicious_patterns(path: str) -> bool: """ Check for suspicious patterns that might indicate malicious intent.