Browse Source

Fix stored XSS vulnerabilities and unauthenticated auth toggle

  - Sanitize project notes with DOMPurify before rendering via
    dangerouslySetInnerHTML (ProjectDetailPage.tsx)
  - Replace hand-rolled HTML sanitizer with DOMPurify in ProjectPageModal
    to prevent attribute injection via crafted 3MF href values
  - Block /api/v1/auth/setup when auth is already enabled to prevent
    unauthenticated clients from disabling authentication remotely
maziggy 2 months ago
parent
commit
fa6edfbcde

+ 3 - 0
CHANGELOG.md

@@ -35,6 +35,9 @@ All notable changes to Bambuddy will be documented in this file.
 - **Spool Assignment Applies Wrong Filament Profile** ([#681](https://github.com/maziggy/bambuddy/issues/681)) — Assigning a spool with a specific filament variant (e.g. "Generic PLA Silk") to an AMS slot applied the base profile instead (e.g. "Generic PLA"). The Bambu Cloud API returns only the base `filament_id` for versioned setting IDs (`GFSL99` → `GFL99`), ignoring variant suffixes (`GFSL99_01`). Added a cross-check that compares the resolved filament name against the spool's stored preset name and corrects the filament ID via reverse lookup when they don't match (e.g. `GFL99` → `GFL96` for "Generic PLA Silk"). Reported by @peter-k-de.
 
 ### Security
+- **Stored XSS via Project Notes** — Project notes were rendered with `dangerouslySetInnerHTML` without sanitization, allowing injected `<script>` or event handler payloads to execute in any viewer's browser and steal JWT tokens from localStorage. Now sanitized with DOMPurify before rendering.
+- **Stored XSS via 3MF Description (Sanitizer Bypass)** — The hand-rolled HTML sanitizer in the Project Page modal reconstructed `<a>` tags by interpolating the `href` attribute without escaping embedded quotes. A crafted 3MF file with a single-quoted `href` containing a double-quote break-out could inject `onmouseover` event handlers through the sanitizer. Replaced the custom sanitizer with DOMPurify.
+- **Unauthenticated Auth Toggle via Setup Endpoint** — The `/api/v1/auth/setup` endpoint could be called without authentication even when auth was already enabled, allowing any network client to disable authentication entirely. Now returns 403 when auth is already enabled; use the authenticated admin panel to modify auth settings.
 - **PyJWT ≥2.12.0** — Bumped minimum version to address CVE-2026-32597.
 - **flatted ≥3.4.0** — Updated transitive ESLint dependency to address GHSA-25h7-pfq9-p65f (unbounded recursion DoS).
 - **Access Code Redacted from Support Logs** — Printer access codes embedded in RTSP stream URLs were not redacted in support bundles and bug report logs. Extended the URL credential sanitizer to cover `rtsps://` URLs and added access codes to the sensitive string collection for exact-match redaction.

+ 7 - 1
backend/app/api/routes/auth.py

@@ -157,7 +157,13 @@ async def setup_auth(request: SetupRequest, db: AsyncSession = Depends(get_db)):
     logger = logging.getLogger(__name__)
 
     try:
-        # If auth_enabled is true but no users exist, allow re-setup (recovery scenario)
+        # If auth is currently enabled, block unauthenticated setup changes.
+        # Use the admin panel (/disable endpoint) to modify auth when it's already on.
+        if await is_auth_enabled(db):
+            raise HTTPException(
+                status_code=status.HTTP_403_FORBIDDEN,
+                detail="Authentication is already configured. Use the admin panel to modify auth settings.",
+            )
 
         admin_created = False
 

+ 15 - 0
frontend/package-lock.json

@@ -22,6 +22,7 @@
         "@tiptap/react": "^3.11.1",
         "@tiptap/starter-kit": "^3.11.1",
         "@types/three": "^0.181.0",
+        "dompurify": "^3.3.3",
         "gcode-preview": "^2.18.0",
         "i18next": "25.6.3",
         "i18next-browser-languagedetector": "^8.2.0",
@@ -3071,6 +3072,12 @@
         "meshoptimizer": "~0.22.0"
       }
     },
+    "node_modules/@types/trusted-types": {
+      "version": "2.0.7",
+      "resolved": "https://registry.npmjs.org/@types/trusted-types/-/trusted-types-2.0.7.tgz",
+      "integrity": "sha512-ScaPdn1dQczgbl0QFTeTOmVHFULt394XJgOQNoyVhZ6r2vLnMLJfBPd53SB52T/3G36VI1/g2MZaX0cwDuXsfw==",
+      "optional": true
+    },
     "node_modules/@types/use-sync-external-store": {
       "version": "0.0.6",
       "resolved": "https://registry.npmjs.org/@types/use-sync-external-store/-/use-sync-external-store-0.0.6.tgz",
@@ -4261,6 +4268,14 @@
       "license": "MIT",
       "peer": true
     },
+    "node_modules/dompurify": {
+      "version": "3.3.3",
+      "resolved": "https://registry.npmjs.org/dompurify/-/dompurify-3.3.3.tgz",
+      "integrity": "sha512-Oj6pzI2+RqBfFG+qOaOLbFXLQ90ARpcGG6UePL82bJLtdsa6CYJD7nmiU8MW9nQNOtCHV3lZ/Bzq1X0QYbBZCA==",
+      "optionalDependencies": {
+        "@types/trusted-types": "^2.0.7"
+      }
+    },
     "node_modules/dunder-proto": {
       "version": "1.0.1",
       "resolved": "https://registry.npmjs.org/dunder-proto/-/dunder-proto-1.0.1.tgz",

+ 1 - 0
frontend/package.json

@@ -28,6 +28,7 @@
     "@tiptap/react": "^3.11.1",
     "@tiptap/starter-kit": "^3.11.1",
     "@types/three": "^0.181.0",
+    "dompurify": "^3.3.3",
     "gcode-preview": "^2.18.0",
     "i18next": "25.6.3",
     "i18next-browser-languagedetector": "^8.2.0",

+ 7 - 46
frontend/src/components/ProjectPageModal.tsx

@@ -1,4 +1,5 @@
 import { useState, useEffect } from 'react';
+import DOMPurify from 'dompurify';
 import { useQuery, useMutation, useQueryClient } from '@tanstack/react-query';
 import {
   X,
@@ -93,53 +94,13 @@ export function ProjectPageModal({ archiveId, archiveName, onClose }: ProjectPag
     setEditData({});
   };
 
-  // Sanitize HTML content (basic XSS prevention)
+  // Sanitize HTML content using DOMPurify
   const sanitizeHtml = (html: string) => {
-    // Allow basic formatting tags only
-    const allowed = ['p', 'br', 'b', 'strong', 'i', 'em', 'u', 'a', 'ul', 'ol', 'li', 'figure', 'img'];
-    const doc = new DOMParser().parseFromString(html, 'text/html');
-
-    const clean = (node: Node): string => {
-      if (node.nodeType === Node.TEXT_NODE) {
-        return node.textContent || '';
-      }
-      if (node.nodeType === Node.ELEMENT_NODE) {
-        const el = node as Element;
-        const tag = el.tagName.toLowerCase();
-
-        if (!allowed.includes(tag)) {
-          // Return children content without the tag
-          return Array.from(el.childNodes).map(clean).join('');
-        }
-
-        // Build allowed attributes
-        let attrs = '';
-        if (tag === 'a' && el.getAttribute('href')) {
-          const href = el.getAttribute('href');
-          if (href?.toLowerCase().startsWith('http')) {
-            attrs = ` href="${href}" target="_blank" rel="noopener noreferrer"`;
-          }
-        }
-        if (tag === 'img') {
-          const src = el.getAttribute('src');
-          // Only render img if it has a valid http(s) URL, otherwise skip entirely
-          if (!src?.toLowerCase().startsWith('http')) {
-            return ''; // Skip images without valid URLs
-          }
-          attrs = ` src="${src}" style="max-width: 100%; height: auto;"`;
-        }
-
-        const children = Array.from(el.childNodes).map(clean).join('');
-
-        if (['br', 'img'].includes(tag)) {
-          return `<${tag}${attrs} />`;
-        }
-        return `<${tag}${attrs}>${children}</${tag}>`;
-      }
-      return '';
-    };
-
-    return Array.from(doc.body.childNodes).map(clean).join('');
+    return DOMPurify.sanitize(html, {
+      ALLOWED_TAGS: ['p', 'br', 'b', 'strong', 'i', 'em', 'u', 'a', 'ul', 'ol', 'li', 'figure', 'img'],
+      ALLOWED_ATTR: ['href', 'src', 'target', 'rel', 'style'],
+      ADD_ATTR: ['target'],
+    });
   };
 
   const hasContent = projectPage && (

+ 2 - 1
frontend/src/pages/ProjectDetailPage.tsx

@@ -1,4 +1,5 @@
 import { useState } from 'react';
+import DOMPurify from 'dompurify';
 import { useParams, useNavigate, Link } from 'react-router-dom';
 import { useQuery, useMutation, useQueryClient } from '@tanstack/react-query';
 import { useTranslation } from 'react-i18next';
@@ -807,7 +808,7 @@ export function ProjectDetailPage() {
           ) : project.notes ? (
             <div
               className="prose prose-invert prose-sm max-w-none"
-              dangerouslySetInnerHTML={{ __html: project.notes }}
+              dangerouslySetInnerHTML={{ __html: DOMPurify.sanitize(project.notes) }}
             />
           ) : (
             <p className="text-bambu-gray/70 text-sm italic">

File diff suppressed because it is too large
+ 0 - 0
static/assets/index-CyeSJFXC.js


+ 1 - 1
static/index.html

@@ -23,7 +23,7 @@
 
     <!-- Splash screens for iOS -->
     <link rel="apple-touch-startup-image" href="/img/android-chrome-512x512.png" />
-    <script type="module" crossorigin src="/assets/index-jwN56PpH.js"></script>
+    <script type="module" crossorigin src="/assets/index-CyeSJFXC.js"></script>
     <link rel="stylesheet" crossorigin href="/assets/index-BgeMokkR.css">
   </head>
   <body>

Some files were not shown because too many files changed in this diff