Procházet zdrojové kódy

Fix API key empty printer_ids granting full access

  An API key with printer_ids=[] was treated the same as null (global
  access) due to a falsy check. Now None means global access and []
  means no printer access. Added a startup migration to normalize any
  existing [] rows to NULL so they retain their intended global access.

  Also fixed the webhook /queue endpoint which used the same falsy
  check, allowing []-scoped keys to see all printers.
maziggy před 1 měsícem
rodič
revize
70b12e1949

+ 1 - 0
CHANGELOG.md

@@ -18,6 +18,7 @@ All notable changes to Bambuddy will be documented in this file.
 ### 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.
 - **Unauthenticated Bug Report Endpoints** — The bug report endpoints (`/start-logging`, `/stop-logging`, `/submit`) had no authentication, allowing anyone on the network to enable debug logging, retrieve system logs, and trigger bug report submissions with system diagnostics when authentication was enabled. All three endpoints now require authentication — `start-logging` requires `settings:update` permission, `stop-logging` and `submit` require `settings:read`. Endpoints remain open when authentication is disabled (the default). Reported responsibly by Sacha Vaudey via security@bambuddy.cool.
+- **API Key Empty Printer List Grants Full Access** — An API key with an empty `printer_ids` list (`[]`) was treated identically to `null` (global access to all printers), granting full printer access instead of no access. Now `null` means global access (admin key) and `[]` means no printer access. Existing API keys with empty lists are automatically migrated to `null` on startup. Also fixed the webhook queue endpoint which used a falsy check that would bypass the filter for empty lists. 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.

+ 1 - 1
backend/app/api/routes/webhook.py

@@ -294,7 +294,7 @@ async def webhook_get_queue_status(
         result = await db.execute(select(Printer))
         printers = result.scalars().all()
         # Filter by allowed printers if limited
-        if api_key.printer_ids:
+        if api_key.printer_ids is not None:
             printers = [p for p in printers if p.id in api_key.printer_ids]
 
     response = []

+ 3 - 3
backend/app/core/auth.py

@@ -547,11 +547,11 @@ def check_printer_access(api_key: APIKey, printer_id: int) -> None:
     Raises:
         HTTPException: If access is denied
     """
-    # If printer_ids is None or empty, access to all printers
-    if api_key.printer_ids is None or len(api_key.printer_ids) == 0:
+    # None = global key, access to all printers
+    if api_key.printer_ids is None:
         return
 
-    # Check if printer_id is in allowed list
+    # Empty list or printer not in allowed list = no access
     if printer_id not in api_key.printer_ids:
         raise HTTPException(
             status_code=status.HTTP_403_FORBIDDEN,

+ 4 - 0
backend/app/core/database.py

@@ -1319,6 +1319,10 @@ async def run_migrations(conn):
         except (OperationalError, ProgrammingError):
             pass  # Already applied
 
+    # Migration: Normalize empty printer_ids [] to NULL (global access) on API keys
+    # Previously both None and [] meant "all printers"; now [] means "no printers"
+    await _safe_execute(conn, "UPDATE api_keys SET printer_ids = NULL WHERE printer_ids = '[]'")
+
     # Seed default settings keys that must exist on fresh install
     default_settings = [
         ("advanced_auth_enabled", "false"),