mirror of
https://github.com/makeplane/plane.git
synced 2026-06-13 19:19:54 +00:00
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
This commit is contained in:
committed by
GitHub
parent
45b4fc8932
commit
aea66f53f4
@@ -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")
|
||||
|
||||
@@ -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")
|
||||
|
||||
@@ -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"),
|
||||
|
||||
@@ -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))
|
||||
|
||||
|
||||
@@ -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}"
|
||||
|
||||
@@ -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}"
|
||||
|
||||
|
||||
|
||||
@@ -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", "")
|
||||
|
||||
@@ -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.
|
||||
|
||||
Reference in New Issue
Block a user