fix(files): allow uploads by extension allowlist, serve unsafe types as downloads

baseapp/files/README.md

@@ -820,27 +820,48 @@ def _validate_file_params(self, file_size, num_parts, part_size, user):
 
 ### File Type Restrictions
 
-Restrict allowed file types:
+Only extensions on an allowlist can be uploaded (`baseapp/files/content_safety.py`). The
+default covers common media, documents and archives:
+
+- Images: `.jpg .jpeg .png .gif .webp .avif .heic .heif .bmp .tif .tiff`
+- Video: `.mp4 .m4v .mov .webm .mkv .avi .3gp`
+- Audio: `.mp3 .m4a .aac .wav .ogg .oga .opus .flac`
+- Documents: `.pdf .doc .docx .xls .xlsx .ppt .pptx .odt .ods .odp .rtf .txt .csv`
+- Archives: `.zip .7z .rar .tar .gz`
+
+Anything else is rejected when the upload starts:
+
+```json
+400 {"file_name": ["Files of type .html are not allowed."]}
+```
+
+The list maps each extension to the content type the file is **stored and served with**; the
+type the client declares is ignored. Otherwise `photo.png` declared as `text/html` would be
+served as a page and run script on the site's domain (stored XSS), even from an S3 bucket
+behind a subdomain.
+
+On S3, files a browser would not display safely (everything but common images, video, audio
+and PDF) are stored with `Content-Disposition: attachment`, so opening them downloads rather
+than renders them. Embedded images and video still display.
+
+Replace or extend the list per project:
 
 ```python
-# In settings.py
-ALLOWED_FILE_TYPES = [
-    'image/jpeg',
-    'image/png',
-    'video/mp4',
-    'application/pdf',
-]
+from baseapp.files.content_safety import DEFAULT_ALLOWED_FILE_TYPES
 
-# In InitiateUploadSerializer.validate_file_content_type
-def validate_file_content_type(self, value):
-    allowed = getattr(settings, 'ALLOWED_FILE_TYPES', None)
-    if allowed and value not in allowed:
-        raise serializers.ValidationError(
-            f"File type {value} not allowed. Allowed types: {', '.join(allowed)}"
-        )
-    return value
+# Extend: keep the defaults and add more types.
+BASEAPP_FILES_ALLOWED_FILE_TYPES = {
+    **DEFAULT_ALLOWED_FILE_TYPES,
+    ".dwg": "application/acad",
+}
+
+# Or replace: a list of extensions takes each type from `mimetypes`.
+BASEAPP_FILES_ALLOWED_FILE_TYPES = [".jpg", ".png", ".pdf"]
 ```
 
+Avoid allowing `.svg`, `.html`, `.xml` or `.js`: browsers run them as pages, and where media is
+served from the site's own domain (local storage, nginx `/media/`) that is stored XSS.
+
 ### Progress Notifications
 
 Add real-time progress updates via WebSockets:

baseapp/files/UPLOAD_FLOW.md

@@ -190,6 +190,7 @@ Without it uploads fail in the browser with a CORS error and nothing reaches the
 
 ### Common Errors
 - **400 Bad Request**: Invalid parameters, part numbers, or file size
+- **400 Bad Request** `{"file_name": ["Files of type .html are not allowed."]}`: only extensions on `BASEAPP_FILES_ALLOWED_FILE_TYPES` (media, documents and archives by default) can be uploaded, and the file is stored with the type that list gives, never the declared one. See the README's "File Type Restrictions".
 - **400 Bad Request** on complete: the assembled file's size differs from the declared `file_size` (e.g. a part carried extra bytes). The stored object is deleted and the upload is not completed.
 - **403 Forbidden**: User doesn't own the file/upload
 - **404 Not Found**: File or upload not found

baseapp/files/content_safety.py

@@ -0,0 +1,124 @@
+"""
+Which files may be uploaded, and how they are served.
+
+Only extensions on an allowlist are accepted, so every uploadable type is a deliberate
+choice. The type a file is stored and served with comes from that list, never from the
+client: a declared ``text/html`` on ``photo.png`` would otherwise be served as a page and
+run script on the site's domain (stored XSS), including from an S3 bucket behind a
+subdomain. Stored objects that a browser would not display safely are served as downloads
+(``Content-Disposition: attachment``).
+"""
+
+import mimetypes
+import posixpath
+from typing import Mapping
+
+from django.conf import settings
+
+IMAGE_JPEG = "image/jpeg"
+IMAGE_PNG = "image/png"
+
+# Extension -> the content type the file is stored and served with. No SVG, HTML, XML or
+# JavaScript: browsers run those as pages.
+DEFAULT_ALLOWED_FILE_TYPES: Mapping[str, str] = {
+    # Images
+    ".jpg": IMAGE_JPEG,
+    ".jpeg": IMAGE_JPEG,
+    ".png": IMAGE_PNG,
+    ".gif": "image/gif",
+    ".webp": "image/webp",
+    ".avif": "image/avif",
+    ".heic": "image/heic",
+    ".heif": "image/heif",
+    ".bmp": "image/bmp",
+    ".tif": "image/tiff",
+    ".tiff": "image/tiff",
+    # Video
+    ".mp4": "video/mp4",
+    ".m4v": "video/x-m4v",
+    ".mov": "video/quicktime",
+    ".webm": "video/webm",
+    ".mkv": "video/x-matroska",
+    ".avi": "video/x-msvideo",
+    ".3gp": "video/3gpp",
+    # Audio
+    ".mp3": "audio/mpeg",
+    ".m4a": "audio/mp4",
+    ".aac": "audio/aac",
+    ".wav": "audio/wav",
+    ".ogg": "audio/ogg",
+    ".oga": "audio/ogg",
+    ".opus": "audio/opus",
+    ".flac": "audio/flac",
+    # Documents
+    ".pdf": "application/pdf",
+    ".doc": "application/msword",
+    ".docx": "application/vnd.openxmlformats-officedocument.wordprocessingml.document",
+    ".xls": "application/vnd.ms-excel",
+    ".xlsx": "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet",
+    ".ppt": "application/vnd.ms-powerpoint",
+    ".pptx": "application/vnd.openxmlformats-officedocument.presentationml.presentation",
+    ".odt": "application/vnd.oasis.opendocument.text",
+    ".ods": "application/vnd.oasis.opendocument.spreadsheet",
+    ".odp": "application/vnd.oasis.opendocument.presentation",
+    ".rtf": "application/rtf",
+    ".txt": "text/plain",
+    ".csv": "text/csv",
+    # Archives
+    ".zip": "application/zip",
+    ".7z": "application/x-7z-compressed",
+    ".rar": "application/vnd.rar",
+    ".tar": "application/x-tar",
+    ".gz": "application/gzip",
+}
+
+# Types a browser displays without running page script; anything else is a download.
+INLINE_CONTENT_TYPES = (
+    IMAGE_PNG,
+    IMAGE_JPEG,
+    "image/gif",
+    "image/webp",
+    "image/avif",
+    "image/bmp",
+    "application/pdf",
+)
+INLINE_CONTENT_TYPE_PREFIXES = ("video/", "audio/")
+
+
+def file_extension(file_name: str) -> str:
+    """Lower-cased extension of the base name, ignoring trailing dots and spaces."""
+    basename = posixpath.basename((file_name or "").replace("\\", "/")).rstrip(". ")
+    return posixpath.splitext(basename)[1].lower()
+
+
+def normalize_content_type(content_type: str) -> str:
+    """``"Text/HTML; charset=utf-8"`` -> ``"text/html"``."""
+    return (content_type or "").split(";", 1)[0].strip().lower()
+
+
+def allowed_file_types() -> dict[str, str]:
+    """
+    ``BASEAPP_FILES_ALLOWED_FILE_TYPES`` as extension -> content type. A dict is used as
+    is; a list of extensions gets each type from ``mimetypes``.
+    """
+    configured = getattr(settings, "BASEAPP_FILES_ALLOWED_FILE_TYPES", DEFAULT_ALLOWED_FILE_TYPES)
+    if isinstance(configured, Mapping):
+        return {ext.lower(): content_type for ext, content_type in configured.items()}
+    return {
+        ext.lower(): mimetypes.guess_type(f"file{ext}")[0] or "application/octet-stream"
+        for ext in configured
+    }
+
+
+def allowed_content_type(file_name: str) -> str | None:
+    """The content type to store ``file_name`` with, or None when its extension is not allowed."""
+    extension = file_extension(file_name)
+    if not extension:
+        return None
+    return allowed_file_types().get(extension)
+
+
+def is_inline_safe(content_type: str) -> bool:
+    """Whether a browser may display this type inline instead of downloading it."""
+    normalized = normalize_content_type(content_type)
+    return normalized in INLINE_CONTENT_TYPES or normalized.startswith(INLINE_CONTENT_TYPE_PREFIXES)

baseapp/files/rest_framework/files/serializers.py

@@ -41,6 +41,9 @@ class Meta:
         read_only_fields = [
             "id",
             "relay_id",
+            # Fixed at upload: the stored file is named and typed after them (allowlist).
+            "file_name",
+            "file_content_type",
             "upload_status",
             "created_by",
             "created",

baseapp/files/rest_framework/uploads/serializers.py

@@ -6,6 +6,8 @@
 
 from baseapp_core.models import DocumentId
 
+from ...content_safety import allowed_content_type, file_extension
+
 File = swapper.load_model("baseapp_files", "File")
 
 
@@ -14,13 +16,26 @@ class InitiateUploadSerializer(serializers.Serializer):
 
     file_name = serializers.CharField(max_length=512)
     file_size = serializers.IntegerField(min_value=1)
-    file_content_type = serializers.CharField(max_length=150)
+    # Only informative: the allowlist decides the stored type, so blank (a browser that
+    # doesn't know the type) or missing is fine.
+    file_content_type = serializers.CharField(max_length=150, required=False, allow_blank=True)
     num_parts = serializers.IntegerField(min_value=1, max_value=10000)
     part_size = serializers.IntegerField(min_value=1)
 
     # Optional parent relationship via DocumentId public_id
     parent_id = serializers.UUIDField(required=False, allow_null=True)
 
+    def validate_file_name(self, value: str) -> str:
+        """Only extensions on BASEAPP_FILES_ALLOWED_FILE_TYPES may be uploaded."""
+        if allowed_content_type(value) is None:
+            extension = file_extension(value)
+            if extension:
+                raise serializers.ValidationError(
+                    _("Files of type %(extension)s are not allowed.") % {"extension": extension}
+                )
+            raise serializers.ValidationError(_("This file type is not allowed."))
+        return value
+
     def validate_file_size(self, value: int) -> int:
         """Reject a declared size above MAX_FILE_UPLOAD_SIZE, naming the limit.
 
@@ -54,7 +69,11 @@ def validate_parent_id(self, value):
             ) from None
 
     def validate(self, data):
-        """Validate file size matches parts."""
+        """Validate file size matches parts, and store the type the extension allows."""
+        # The declared type is the client's word; a page type on an allowed extension would
+        # be served as a page. The allowlist decides how the file is stored and served.
+        data["file_content_type"] = allowed_content_type(data["file_name"])
+
         num_parts = data["num_parts"]
         part_size = data["part_size"]
         file_size = data["file_size"]

baseapp/files/services/upload_service.py

@@ -6,6 +6,11 @@
 from django.db import transaction
 from django.utils import timezone
 
+from ..content_safety import (
+    allowed_content_type,
+    file_extension,
+    normalize_content_type,
+)
 from ..storage import get_upload_handler
 
 logger = logging.getLogger(__name__)
@@ -43,7 +48,8 @@ def initiate_multipart_upload(
                 ``created_by`` empty
             file_name: Original file name
             file_size: Size in bytes
-            file_content_type: MIME type
+            file_content_type: MIME type the client declared; replaced by the type
+                BASEAPP_FILES_ALLOWED_FILE_TYPES gives the extension
             num_parts: Number of parts for multipart upload
             part_size: Size of each part in bytes
             parent_id: DocumentId primary key (optional)
@@ -55,12 +61,23 @@ def initiate_multipart_upload(
 
         # Validate inputs
         self._validate_file_params(file_size, num_parts, part_size)
+        stored_content_type = allowed_content_type(file_name)
+        if stored_content_type is None:
+            raise ValueError("File type is not allowed")
+        if file_content_type and normalize_content_type(file_content_type) != stored_content_type:
+            # The extension only: file names can carry personal information.
+            logger.info(
+                "Upload with extension %r declared %r; storing it as %r",
+                file_extension(file_name),
+                file_content_type,
+                stored_content_type,
+            )
 
         # Create File record in pending state
         file_obj = File.objects.create(
             file_name=file_name,
             file_size=file_size,
-            file_content_type=file_content_type,
+            file_content_type=stored_content_type,
             upload_status=File.UploadStatus.PENDING,
             total_parts=num_parts,
             created_by=user if user and user.is_authenticated else None,
@@ -107,6 +124,11 @@ def complete_multipart_upload(self, file_id: int, parts: list):
         # Validate parts
         self._validate_parts(file_obj, parts)
 
+        # The local handler names the stored file after file_name; re-check it here in
+        # case it changed since initiation, including to another allowed type.
+        if allowed_content_type(file_obj.file_name) != file_obj.file_content_type:
+            raise ValueError("File type is not allowed")
+
         # Complete with storage handler (network I/O — outside the transaction)
         try:
             file_path = self.handler.complete_upload(file_obj, file_obj.upload_id, parts)

baseapp/files/storage/s3.py

@@ -6,7 +6,9 @@
 from django.conf import settings
 from django.core.files.storage import Storage, default_storage
 from django.utils.crypto import salted_hmac
+from django.utils.http import content_disposition_header
 
+from ..content_safety import is_inline_safe
 from .base import BaseUploadHandler
 
 if TYPE_CHECKING:
@@ -136,10 +138,19 @@ def _get_write_parameters(self, file_obj: "AbstractFile") -> Dict[str, Any]:
         the storage's ``get_object_parameters()`` (``AWS_S3_OBJECT_PARAMETERS``: ACL,
         CacheControl, server-side encryption, ...), and ``default_acl`` only when those set no
         ACL. Like ``_save``, ``get_object_parameters`` receives the full, location-prefixed key.
+
+        Types a browser would not display safely get ``ContentDisposition: attachment``
+        (``content_safety.is_inline_safe``); the storage's object parameters may override it.
         """
         params: Dict[str, Any] = {
             "ContentType": file_obj.file_content_type or "application/octet-stream"
         }
+        # Served from a custom domain, a type the browser would render could run script
+        # there; anything not known to display safely is a download instead.
+        if not is_inline_safe(params["ContentType"]):
+            params["ContentDisposition"] = content_disposition_header(
+                as_attachment=True, filename=file_obj.file_name or "file"
+            )
         get_object_parameters = getattr(self.storage, "get_object_parameters", None)
         if get_object_parameters:
             params.update(get_object_parameters(self._get_s3_key(file_obj)))