From 7fa00ed475ad8340a3e8be737ddfe31c07221f46 Mon Sep 17 00:00:00 2001 From: maziggy Date: Sat, 4 Apr 2026 12:41:01 +0200 Subject: [PATCH] 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. --- CHANGELOG.md | 3 +++ backend/app/api/routes/archives.py | 38 ++++++++++++++++++++---------- 2 files changed, 29 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index aff61131a..4a44cd4c9 100644 --- a/CHANGELOG.md +++ b/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. diff --git a/backend/app/api/routes/archives.py b/backend/app/api/routes/archives.py index 07f8661a1..c0b420b86 100644 --- a/backend/app/api/routes/archives.py +++ b/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()