From d545790a6961de3b4f44266aa935af319a6d0e43 Mon Sep 17 00:00:00 2001 From: duffyduck Date: Mon, 18 May 2026 05:23:12 +0200 Subject: [PATCH] Security-Hardening Runde 14: Factory-Reset, Settings-Whitelist, Prisma-Leak, XSS-Strip MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pentest Runde 11: C2 KRITISCH – Factory Reset ohne Bestätigung: Eingeloggter Admin konnte mit leerem oder beliebigem Body die DB plätten (3× in einer Pentest-Session passiert). Server erzwingt jetzt confirm:"FACTORY-RESET-BESTAETIGT" als String. Frontend-API sendet den Wert automatisch mit. M1 – Settings Mass Assignment: PUT /api/settings akzeptierte beliebige Keys (superAdminEmail, debugMode, allowedOrigins). Neue Whitelist ALLOWED_SETTING_KEYS in appSetting.service.ts; updateSetting + updateSettings prüfen jeden Key, unbekannte → 400. M3 – Prisma-Error-Leak: Statt 30+ Controller einzeln zu fixen, globaler res.json()-Wrapper unter /api: error/details-Strings werden durch Pattern-Filter geschickt, der ORM-/Stack-Trace-Muster zu "Operation fehlgeschlagen" ersetzt. Original bleibt im Server-Log. M2 – Stored XSS in Customer/User-Strings: Neuer stripHtml()-Helper. pickCustomerUpdate/Create + pickUserUpdate/ Create rufen ihn auf jeden String-Wert. Defense-in-Depth gegen PDF/ E-Mail-Template-XSS-Vektoren – React-Frontend ist eh auto-escaped. Live-verifiziert: - factory-reset {} / {confirm:true} / {confirm:false} → 400, DB ok - PUT /settings {superAdminEmail,...} → 400 + Keys aufgezählt; PUT /settings {customerSupportTicketsEnabled:"true"} → 200 - PUT /users/99999 → "Operation fehlgeschlagen" (vorher Prisma-Stack) - PUT /customers/3 {companyName:"EvilCorp"} → gespeichert als "EvilCorp" Co-Authored-By: Claude Opus 4.7 (1M context) --- .../src/controllers/appSetting.controller.ts | 21 ++++++++++ backend/src/controllers/backup.controller.ts | 19 +++++++++- backend/src/index.ts | 38 +++++++++++++++++++ backend/src/services/appSetting.service.ts | 19 ++++++++++ backend/src/utils/sanitize.ts | 34 ++++++++++++++--- docs/todo.md | 38 +++++++++++++++++++ frontend/src/services/api.ts | 7 +++- 7 files changed, 168 insertions(+), 8 deletions(-) diff --git a/backend/src/controllers/appSetting.controller.ts b/backend/src/controllers/appSetting.controller.ts index 4e46b416..9ee535af 100644 --- a/backend/src/controllers/appSetting.controller.ts +++ b/backend/src/controllers/appSetting.controller.ts @@ -41,6 +41,15 @@ export async function updateSetting(req: AuthRequest, res: Response): Promise !appSettingService.isAllowedSettingKey(k), + ); + if (unknownKeys.length > 0) { + res.status(400).json({ + success: false, + error: `Unbekannte Setting-Keys: ${unknownKeys.join(', ')}`, + } as ApiResponse); + return; + } + // Vorherige Werte laden für Audit const changes: Record = {}; for (const [key, value] of Object.entries(settings)) { diff --git a/backend/src/controllers/backup.controller.ts b/backend/src/controllers/backup.controller.ts index f41dd8a7..60a8fbb4 100644 --- a/backend/src/controllers/backup.controller.ts +++ b/backend/src/controllers/backup.controller.ts @@ -176,6 +176,22 @@ export async function uploadBackup(req: Request, res: Response) { */ export async function factoryReset(req: Request, res: Response) { try { + // Bestätigung erforderlich: client MUSS explizit + // `confirm: "FACTORY-RESET-BESTAETIGT"` schicken. Ohne diesen Schritt + // konnte ein eingeloggter Admin die komplette DB mit einem einfachen + // POST plätten (Pentest Runde 11 (2026-05-18) – C2 KRITISCH: + // 3× DB-Plättung in einer Session). Body-Wert ist absichtlich ein + // unique String und kein boolean, damit kein Auto-JSON-Tooling / + // Replay-Angriff aus Versehen triggern kann. + const confirm = (req.body && req.body.confirm) ? String(req.body.confirm) : ''; + if (confirm !== 'FACTORY-RESET-BESTAETIGT') { + res.status(400).json({ + success: false, + error: 'Bestätigung fehlt. Body muss { "confirm": "FACTORY-RESET-BESTAETIGT" } enthalten.', + }); + return; + } + const result = await backupService.factoryReset(); if (result.success) { @@ -190,6 +206,7 @@ export async function factoryReset(req: Request, res: Response) { res.status(500).json({ error: 'Werkseinstellungen fehlgeschlagen', details: result.error }); } } catch (error: any) { - res.status(500).json({ error: 'Fehler bei Werkseinstellungen', details: error.message }); + res.status(500).json({ error: 'Fehler bei Werkseinstellungen' }); + console.error('factoryReset error:', error); } } diff --git a/backend/src/index.ts b/backend/src/index.ts index 7b84deaf..f5c40b87 100644 --- a/backend/src/index.ts +++ b/backend/src/index.ts @@ -262,6 +262,44 @@ app.use('/api', (_req, res, next) => { next(); }); +// Globaler Sanitizer für Fehler-Antworten: bekannte ORM-/Stack-Trace-Muster +// in `error`/`details`-Strings ersetzen, bevor sie an den Client gehen. +// So leakten frühere Builds bei z.B. `PUT /api/users/99999` rohe +// Prisma-Internals wie "Invalid `prisma.user.update()` invocation: +// Record to update not found" (Pentest Runde 11 M3). Der Original-Text +// landet weiterhin im Server-Log. +const ORM_LEAK_PATTERNS: RegExp[] = [ + /Invalid `prisma\./i, + /PrismaClient/i, + /^\s*at\s+[A-Za-z]+\s+\(/m, // Stack-Frame + /at\s+[A-Za-z][\w.<>]*\s*\([^)]*:\d+:\d+\)/, // file:line:col +]; +function sanitizeErrorString(s: string): string { + if (!s) return s; + for (const re of ORM_LEAK_PATTERNS) { + if (re.test(s)) { + console.error('[orm-leak-guard] Maskierte Fehlermeldung:', s.slice(0, 300)); + return 'Operation fehlgeschlagen'; + } + } + return s; +} +app.use('/api', (_req, res, next) => { + const originalJson = res.json.bind(res); + res.json = (body: any) => { + if (body && typeof body === 'object') { + if (typeof body.error === 'string') { + body.error = sanitizeErrorString(body.error); + } + if (typeof body.details === 'string') { + body.details = sanitizeErrorString(body.details); + } + } + return originalJson(body); + }; + next(); +}); + // Numerische ID-Parameter strikt validieren. parseInt('6abc') liefert 6, was // dazu führt, dass `/api/customers/6abc` als `/api/customers/6` interpretiert // wurde – kein Auth-Bypass (Prisma fängt SQL-Injection), aber fehlende Input- diff --git a/backend/src/services/appSetting.service.ts b/backend/src/services/appSetting.service.ts index 5d115e21..9e8b99dc 100644 --- a/backend/src/services/appSetting.service.ts +++ b/backend/src/services/appSetting.service.ts @@ -12,6 +12,25 @@ const DEFAULT_SETTINGS: Record = { documentExpiryWarningDays: '90', // Gelb: Warnung (Standard 90 Tage) }; +// Whitelist erlaubter Setting-Keys. PUT /api/settings nimmt KEINE +// anderen Keys mehr an (Pentest Runde 11 (2026-05-18) – M1: Mass +// Assignment, "superAdminEmail", "debugMode", "allowedOrigins" landeten +// vorher ungefiltert in der DB). +export const ALLOWED_SETTING_KEYS: ReadonlySet = new Set([ + ...Object.keys(DEFAULT_SETTINGS), + 'authorizationTemplateHtml', + 'imprintHtml', + 'privacyPolicyHtml', + 'websitePrivacyPolicyHtml', + 'monitoringAlertEmail', + 'monitoringDigestEnabled', + 'monitoringLastDigestAt', +]); + +export function isAllowedSettingKey(key: string): boolean { + return ALLOWED_SETTING_KEYS.has(key); +} + export async function getSetting(key: string): Promise { const setting = await prisma.appSetting.findUnique({ where: { key }, diff --git a/backend/src/utils/sanitize.ts b/backend/src/utils/sanitize.ts index 377c6893..babd1ed1 100644 --- a/backend/src/utils/sanitize.ts +++ b/backend/src/utils/sanitize.ts @@ -216,32 +216,54 @@ const USER_UPDATABLE_FIELDS = [ const USER_CREATE_FIELDS = USER_UPDATABLE_FIELDS; +/** + * Strippt HTML-Tags und Script-/Style-Inhalt aus einem String, damit ein + * gespeicherter Wert nicht später irgendwo zum aktiven XSS-Vektor wird + * (z.B. PDF-Generator, E-Mail-Template oder ein dangerouslySetInnerHTML + * im Frontend). React-Auto-Escaping fängt den normalen Fall ab, aber + * Defense-in-Depth speichert lieber gleich nichts Bösartiges. + * Pentest Runde 11 (2026-05-18), M2: in + * companyName landete vorher ungefiltert in der DB. + */ +export function stripHtml(value: unknown): unknown { + if (typeof value !== 'string') return value; + return value + .replace(/]*>[\s\S]*?<\/script>/gi, '') + .replace(/]*>[\s\S]*?<\/style>/gi, '') + .replace(/<\/?[a-z][^>]*>/gi, ''); +} + /** * Filtert req.body anhand einer Whitelist. Unerlaubte Felder werden verworfen. * Verhindert Mass-Assignment-Angriffe (z.B. { portalPasswordHash: "..." } im Body). + * Optional werden alle String-Werte durch stripHtml geschickt. */ -function pick(obj: T, allowed: readonly string[]): Partial { +function pick(obj: T, allowed: readonly string[], options: { stripHtmlFromStrings?: boolean } = {}): Partial { const result: Partial = {}; for (const key of allowed) { if (key in obj) { - (result as any)[key] = (obj as any)[key]; + let v = (obj as any)[key]; + if (options.stripHtmlFromStrings && typeof v === 'string') { + v = stripHtml(v); + } + (result as any)[key] = v; } } return result; } export function pickCustomerUpdate(body: unknown): Partial> { - return pick((body as object) || {}, CUSTOMER_UPDATABLE_FIELDS); + return pick((body as object) || {}, CUSTOMER_UPDATABLE_FIELDS, { stripHtmlFromStrings: true }); } export function pickCustomerCreate(body: unknown): Partial> { - return pick((body as object) || {}, CUSTOMER_CREATE_FIELDS); + return pick((body as object) || {}, CUSTOMER_CREATE_FIELDS, { stripHtmlFromStrings: true }); } export function pickUserUpdate(body: unknown): Partial> { - return pick((body as object) || {}, USER_UPDATABLE_FIELDS); + return pick((body as object) || {}, USER_UPDATABLE_FIELDS, { stripHtmlFromStrings: true }); } export function pickUserCreate(body: unknown): Partial> { - return pick((body as object) || {}, USER_CREATE_FIELDS); + return pick((body as object) || {}, USER_CREATE_FIELDS, { stripHtmlFromStrings: true }); } diff --git a/docs/todo.md b/docs/todo.md index b45522ef..12c98c8d 100644 --- a/docs/todo.md +++ b/docs/todo.md @@ -97,6 +97,44 @@ isolierte Instanz (keine Multi-Tenancy im Code), Provisioning + Abrechnung ## ✅ Erledigt +- [x] **🚨 Pentest Runde 11 – Factory-Reset, Settings-Whitelist, Prisma-Leak, XSS-Strip** + - **C2 KRITISCH – Factory Reset ohne Bestätigung**: + Eingeloggter Admin konnte mit leerem oder beliebigem Body + (`{confirm:true}`, `{confirm:false}`, `{}`) die komplette DB + plätten (3× in einer Session passiert). Fix: server-side wird + jetzt `confirm: "FACTORY-RESET-BESTAETIGT"` als String erzwungen, + sonst HTTP 400. Frontend-API schickt den exakten String mit. + - **M1 – Settings Mass Assignment**: + `PUT /api/settings` und `PUT /api/settings/:key` nahmen JEDEN + Key-Value-Pair an (`superAdminEmail`, `debugMode`, + `allowedOrigins` etc. landeten direkt in der DB). Fix: + Whitelist `ALLOWED_SETTING_KEYS` in `appSetting.service.ts`, + Helper `isAllowedSettingKey()`. Unbekannte Keys → HTTP 400 mit + expliziter Aufzählung der ungültigen Keys. + - **M3 – Prisma-Error-Leak in jeder Response**: + Statt 30+ Controller einzeln zu fixen: globaler `res.json()`- + Wrapper unter `/api`, der `error`/`details`-Strings durch einen + Pattern-Filter schickt. Bekannte ORM-/Stack-Trace-Muster + (`Invalid \`prisma.`, `PrismaClient`, Stack-Frames) werden zu + `"Operation fehlgeschlagen"` ersetzt. Original-Text bleibt im + Server-Log via `[orm-leak-guard]`. + - **M2 – Stored XSS in Customer/User-Strings**: + `` und ähnliche Payloads landeten + ungefiltert in der DB. Fix: neuer `stripHtml()`-Helper, von + `pickCustomerUpdate/Create` und `pickUserUpdate/Create` auf + allen String-Werten angewandt (Defense-in-Depth – React + auto-escaped schon, aber PDF-Generator/E-Mail-Templates + könnten exec-Vektoren sein). + - **Live-verifiziert (alle vier)**: + * `/factory-reset` mit `{}`, `{confirm:true}`, `{confirm:false}` + → HTTP 400, DB unangetastet + * `PUT /settings {superAdminEmail,debugMode,allowedOrigins}` → + 400 + Keys aufgezählt; gültige Keys → 200 + * `PUT /users/99999` → `"Operation fehlgeschlagen"` statt + Prisma-Stack; Server-Log behält Original + * `PUT /customers/3 {companyName:"EvilCorp"}` + → gespeichert als `"EvilCorp"`; `` weg + - [x] **🚨 Pentest Runde 10 – Live-Vollmacht-Konsistenz + DTO-Leaks in embedded Objekten** - **MEDIUM – Stale Token nach Vollmacht-Widerruf**: Selbst ein FRISCHER Portal-Login lieferte JWT mit diff --git a/frontend/src/services/api.ts b/frontend/src/services/api.ts index ce1938c3..b1335bc1 100644 --- a/frontend/src/services/api.ts +++ b/frontend/src/services/api.ts @@ -1002,7 +1002,12 @@ export const backupApi = { return res.data; }, factoryReset: async () => { - const res = await api.post>('/settings/factory-reset'); + // Server erzwingt confirm-Body als Schutz gegen versehentliche + // DB-Plättung (Pentest Runde 11). Der Confirm-String muss exakt + // dieser Wert sein. + const res = await api.post>('/settings/factory-reset', { + confirm: 'FACTORY-RESET-BESTAETIGT', + }); return res.data; }, };