Files
archivmail/features/PROJ-61-fix-tenant-logo-xss-idor.md
sysopsandClaude Sonnet 4.6 363874767b fix(PROJ-61): Cross-Tenant Stored XSS via Mandanten-Logo behoben (Sicherheitsbug)
GET /api/tenants/{id}/logo prüfte nur die Authentifizierung, aber keinen
Tenant-Scope — jeder eingeloggte Nutzer konnte das Logo jedes beliebigen
Tenants lesen. Kombiniert mit dem bisher erlaubten SVG-Upload (kann
eingebettetes JavaScript enthalten) ergab das einen Cross-Tenant Stored-XSS:
ein domain_admin konnte ein bösartiges SVG als eigenes Logo hochladen und
Opfer aus beliebigen anderen Tenants per direktem Link darauf locken.

Fix: tenantAccessAllowed()-Scope-Check beim Logo-Lesepfad (analog PROJ-55),
SVG aus erlaubten Upload-Typen entfernt, X-Content-Type-Options: nosniff
als Defense-in-Depth ergänzt.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-06-25 00:31:41 +02:00

4.9 KiB
Raw Permalink Blame History

PROJ-61: Fix Cross-Tenant Stored XSS via Mandanten-Logo (Sicherheitsbug)

Status: Deployed

Created: 2026-06-25 Last Updated: 2026-06-25

Hintergrund (Security-Audit, 2026-06-25)

Nachtrag zum letzten Audit (docs/security-audit-2026-03-18.md) über alle Commits seit März (PROJ-2560). Zwei kombinierte Lücken in den Tenant-Logo-Handlern (eingeführt mit Multi-Tenancy-Ausbau):

  1. POST /api/tenant/logo (eigener Tenant, domain_admin reicht) akzeptiert image/svg+xml ohne Sanitisierung. SVG kann <script>/onload enthalten.
  2. GET /api/tenants/{id}/logo prüft nur s.auth (jeder eingeloggte Nutzer, egal welcher Tenant/Rolle) — kein Tenant-Scope-Check, anders als die Schreib-/Lösch-Pendants (requireRole/authAdmin prüfen nur Rollenlevel, keinen Tenant-Bezug).

Exploit-Pfad: domain_admin von Tenant A lädt präpariertes SVG als eigenes Logo hoch → lockt ein Opfer aus einem BELIEBIGEN anderen Tenant per direktem Link auf /api/tenants/A/logo → SVG wird same-origin mit image/svg+xml ausgeliefert und im Browser des Opfers ausgeführt → Skript kann mit dem httpOnly-JWT-Cookie des Opfers API-Calls im Namen des Opfers auslösen.

Acceptance Criteria

  • GET /api/tenants/{id}/logo erzwingt Tenant-Scope: nur superadmin/admin (global) ODER der Nutzer gehört zum angefragten Tenant — analog zum projektüblichen tenantAccessAllowed()-Muster (siehe PROJ-55).
  • SVG (image/svg+xml) wird beim Logo-Upload nicht mehr akzeptiert (einfachste, robusteste Lösung).
  • Bestehende, bereits hochgeladene Logos werden beim Ausliefern zusätzlich defensiv abgesichert: X-Content-Type-Options: nosniff Header bei Logo-Responses ergänzt (Defense-in-Depth, auch für nicht-SVG-Typen).
  • Bestehende Funktionalität (PNG/JPEG/GIF/WebP-Logos hoch-/runterladen, eigene + fremde Tenant-Logos für globale Admins) bleibt unverändert nutzbar.
  • Kein Datenverlust: bereits gespeicherte SVG-Logos werden nicht automatisch gelöscht, nur zukünftige Uploads blockiert (siehe Restrisiko unten).

Tech Design

Übersprungen (klar umrissener Sicherheitsfix mit bestehenden Mustern, kein architektonischer Schnitt).

Implementation Notes (2026-06-25)

Geändert: internal/api/tenant_logo_handlers.go, internal/api/ldap_tenants.go.

  1. IDOR-Fix (handleGetTenantLogo): Nach dem Parsen der id wird sessionFromCtx(r.Context()) geholt und tenantAccessAllowed(sess, &id) geprüft. Globale Sessions (sess.TenantID == nil, also superadmin/admin) dürfen jedes Tenant-Logo lesen; alle anderen nur das eigene (*sess.TenantID == id), sonst HTTP 403. Route in ldap_tenants.go bleibt s.auth(...) (setzt Session via authMiddleware), Scope jetzt im Handler erzwungen — Kommentar angepasst. Kein neuer Import nötig (sessionFromCtx/tenantAccessAllowed sind im selben Package).

  2. XSS-Fix (saveTenantLogo): "image/svg+xml" aus der allowed-Map entfernt; Fehlertext und erlaubte Typen aktualisiert (png, jpeg, gif, webp). PNG/JPEG/GIF/WebP unverändert.

  3. Defense-in-Depth: w.Header().Set("X-Content-Type-Options", "nosniff") in handleGetTenantLogo und handleGetOwnTenantLogo vor dem Body-Write ergänzt.

Frontend-Prüfung: getTenantLogoUrl(tenantId) wird ausschließlich in src/hooks/useTenantLogos.ts (Superadmin/Admin-Tenant-Verwaltungsdialog) genutzt; das eigene Logo läuft über /api/tenant/logo. Kein Frontend-Pfad liest fremde Tenant-Logos als nicht-globaler Nutzer → IDOR-Fix bricht nichts.

Restrisiko / Offene Fragen

  • Bestands-SVGs: Bereits gespeicherte SVG-Logos werden weiter mit image/svg+xml ausgeliefert. nosniff verhindert MIME-Sniffing, aber NICHT die Ausführung eines explizit als image/svg+xml ausgelieferten SVG bei direkter Navigation. Restrisiko ist durch den IDOR-Fix stark reduziert (nur noch eigener Tenant + globale Admins). Falls Bestands-SVGs existieren, sollte ein einmaliges Cleanup/Re-Upload erwogen werden (separate Aufgabe).
  • Lokal kein go build möglich — Compile auf 192.168.1.131 (devops-deploy) verifizieren.

QA Test Results (192.168.1.132, 2026-06-25)

  • Build: Exit 0. go vet: keine neuen Befunde (einziger Treffer vorbestehend, PROJ-61-fremd: api_test.go:36 os.Discard).
  • a) superadmin liest beliebiges Tenant-Logo → 200/404, nicht 403. PASS
  • b) domain_admin (Tenant 3) liest fremdes Tenant-Logo (1, 2) → 403. Eigenes Tenant (3) → 404 (kein Logo). PASS — IDOR behoben.
  • c) domain_admin liest eigenes Logo via /api/tenant/logo → normal. PASS
  • d) SVG-Upload → 400 "unsupported image type". PASS — XSS-Vektor blockiert.
  • e) PNG-Upload → 200. PASS — bestehende Funktionalität erhalten.
  • f) X-Content-Type-Options: nosniff im Response-Header bestätigt. PASS
  • Unauthentifizierter Zugriff → 401. Globale Admins weiterhin uneingeschränkt.
  • Keine Regressionen. Cleanup auf 132 durchgeführt, Health-Check OK.

Deployment

  • Test (192.168.1.132): QA bestanden, System nach Test zurückgesetzt. 2026-06-25.
  • Produktion (192.168.1.131): siehe unten.