Kaynağa Gözat

Fix path traversal vulnerability in file upload endpoints

  Archive upload endpoints used the client-supplied filename directly
  in file paths, allowing an authenticated attacker to write files
  outside the intended directory (e.g. ../../evil.3mf bypasses the
  .3mf extension check). Added _safe_filename() helper that normalizes
  backslashes and extracts the basename before constructing paths.
maziggy 1 ay önce
ebeveyn
işleme
7fa00ed475
2 değiştirilmiş dosya ile 29 ekleme ve 12 silme
  1. 3 0
      CHANGELOG.md
  2. 26 12
      backend/app/api/routes/archives.py

+ 3 - 0
CHANGELOG.md

@@ -15,6 +15,9 @@ All notable changes to Bambuddy will be documented in this file.
 - **AMS Drying Support for H2S** ([#886](https://github.com/maziggy/bambuddy/issues/886)) — Remote AMS drying and queue auto-drying now work on H2S printers with firmware 01.02.00.00 or later.
 - **REST Smart Plug: Separate Power/Energy URLs and Unit Multipliers** ([#472](https://github.com/maziggy/bambuddy/issues/472)) — REST/Webhook smart plugs can now use individual URLs for power and energy data instead of requiring all values in a single status response. Each value falls back to the shared Status URL when no separate URL is configured, so existing setups work without changes. Added power and energy multipliers for unit conversion (e.g., set energy multiplier to `0.001` to convert Wh to kWh). Useful for platforms like ioBroker that expose each data point as a separate API endpoint.
 
+### Security
+- **Path Traversal in File Upload Endpoints** — Archive upload endpoints (`/upload`, `/upload-bulk`, `/{id}/source`, `/source-by-name`, `/{id}/f3d`, `/{id}/timelapse`) used the client-supplied filename directly in file paths without stripping directory components. An authenticated attacker could write files outside the intended directory via directory traversal (e.g. `../../evil.3mf`). All upload endpoints now sanitize filenames by extracting only the basename before constructing paths. Reported responsibly by Sacha Vaudey via security@bambuddy.cool.
+
 ### Fixed
 - **Thumbnails Broken After Backend Restart** — Archive and library thumbnails returned 401 Unauthorized after a backend restart because stream tokens are stored in memory and lost on restart. The frontend now detects failed token-protected image loads and automatically refreshes the stream token, so thumbnails recover without a page reload.
 - **SpoolBuddy Kiosk Screen Blanks on Boot** — The touchscreen display would blank immediately after the RPi booted, requiring a touch to wake. Added `consoleblank=0` to the kernel cmdline to disable Linux console blanking during the Plymouth-to-labwc transition, and changed the `wlr-randr` anti-blank loop to fire immediately instead of sleeping 60 seconds first.

+ 26 - 12
backend/app/api/routes/archives.py

@@ -33,6 +33,15 @@ logger = logging.getLogger(__name__)
 router = APIRouter(prefix="/archives", tags=["archives"])
 
 
+def _safe_filename(filename: str) -> str:
+    """Extract basename from a client-supplied filename, preventing path traversal.
+
+    Normalizes backslashes (Windows paths) before extracting so that
+    '..\\\\..\\\\evil.3mf' is correctly stripped to 'evil.3mf' on Linux.
+    """
+    return Path(filename.replace("\\", "/")).name
+
+
 def _validate_user_filter_permission(current_user: User | None, created_by_id: int | None):
     """Raise 403 if created_by_id filter is used without stats:filter_by_user permission."""
     if created_by_id is None or current_user is None:
@@ -1879,12 +1888,13 @@ async def upload_timelapse(
         raise HTTPException(400, "File must be a video file (.mp4, .avi, .mkv)")
 
     content = await file.read()
-    success = await service.attach_timelapse(archive_id, content, file.filename)
+    safe_filename = _safe_filename(file.filename)
+    success = await service.attach_timelapse(archive_id, content, safe_filename)
 
     if not success:
         raise HTTPException(500, "Failed to attach timelapse")
 
-    return {"status": "attached", "filename": file.filename}
+    return {"status": "attached", "filename": safe_filename}
 
 
 @router.get("/{archive_id}/timelapse/info")
@@ -2575,8 +2585,9 @@ async def upload_archive(
     if not file.filename or not file.filename.endswith(".3mf"):
         raise HTTPException(400, "File must be a .3mf file")
 
-    # Save uploaded file temporarily
-    temp_path = settings.archive_dir / "temp" / file.filename
+    # Save uploaded file temporarily — strip directory components to prevent path traversal
+    safe_filename = _safe_filename(file.filename)
+    temp_path = settings.archive_dir / "temp" / safe_filename
     temp_path.parent.mkdir(parents=True, exist_ok=True)
 
     try:
@@ -2615,7 +2626,8 @@ async def upload_archives_bulk(
             errors.append({"filename": file.filename or "unknown", "error": "Not a .3mf file"})
             continue
 
-        temp_path = settings.archive_dir / "temp" / file.filename
+        safe_filename = _safe_filename(file.filename)
+        temp_path = settings.archive_dir / "temp" / safe_filename
         temp_path.parent.mkdir(parents=True, exist_ok=True)
 
         try:
@@ -3311,8 +3323,8 @@ async def upload_source_3mf(
         if old_source_path.exists():
             old_source_path.unlink()
 
-    # Save the source 3MF file - preserve original filename
-    source_filename = file.filename
+    # Save the source 3MF file - preserve original filename, strip directory components
+    source_filename = _safe_filename(file.filename)
     source_path = source_dir / source_filename
 
     content = await file.read()
@@ -3458,10 +3470,12 @@ async def upload_source_3mf_by_name(
     if not file.filename or not file.filename.endswith(".3mf"):
         raise HTTPException(400, "File must be a .3mf file")
 
+    safe_filename = _safe_filename(file.filename)
+
     # Derive print name from filename if not provided
     if not print_name:
         # Remove .3mf extension and common suffixes
-        print_name = file.filename.rsplit(".3mf", 1)[0]
+        print_name = safe_filename.rsplit(".3mf", 1)[0]
         # Remove _source suffix if present
         if print_name.endswith("_source"):
             print_name = print_name[:-7]
@@ -3510,8 +3524,8 @@ async def upload_source_3mf_by_name(
         if old_source_path.exists():
             old_source_path.unlink()
 
-    # Save the source 3MF file - preserve original filename
-    source_filename = file.filename
+    # Save the source 3MF file - preserve original filename, strip directory components
+    source_filename = safe_filename
     source_path = source_dir / source_filename
 
     content = await file.read()
@@ -3591,8 +3605,8 @@ async def upload_f3d(
         if old_f3d_path.exists():
             old_f3d_path.unlink()
 
-    # Save the F3D file - preserve original filename
-    f3d_filename = file.filename
+    # Save the F3D file - preserve original filename, strip directory components
+    f3d_filename = _safe_filename(file.filename)
     f3d_path = f3d_dir / f3d_filename
 
     content = await file.read()