From cf8c6c84c295c0020359d5fcf1ab69337c21f810 Mon Sep 17 00:00:00 2001 From: duffyduck Date: Mon, 18 May 2026 15:09:13 +0200 Subject: [PATCH] Security-Hardening Runde 15: Pentest Runde 12 Folge-Fixes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit M2-Reste – XSS-Strings + Mass-Assignment-Settings noch in DB: Idempotentes Cleanup-Script prisma/cleanup-xss-and-mass-assignment.ts. Strippt HTML aus Customer/User-String-Feldern, entfernt AppSettings ohne Whitelist-Eintrag. Wird im entrypoint.sh nach Migrations + Seed einmalig pro Container-Start ausgeführt. User-Update + password-Feld: password aus USER_UPDATABLE_FIELDS raus (CREATE behält es), neuer dedizierter Endpoint POST /api/users/:id/password mit Audit-Log "Passwort … durch Admin gesetzt" und Komplexitäts-Check. JS-Runtime-Fehler-Leak: ORM_LEAK_PATTERNS um TypeError/ReferenceError/SyntaxError/RangeError + "Cannot read properties of undefined/null" + "is not a function/ defined" erweitert. Greift im globalen res.json()-Wrapper. POST /contracts substring-Crash: Controller validiert type/customerId, sonst 400. generateContractNumber fängt nullish type ab (Fallback "CON"). Seed-Admin-Passwort: Default "admin" verletzte 12-Zeichen-Policy. Jetzt 16-char Zufallspasswort (alle 4 Klassen garantiert via Fisher-Yates) oder per SEED_ADMIN_PASSWORD-ENV überschreibbar. BCRYPT-Cost 12 (war 10). Passwort wird einmalig in stdout ausgegeben mit Warnung. AppSettings-Whitelist: companyName + defaultEmailDomain ergänzt (kamen aus seed.ts, in 1. Whitelist vergessen). Live-verifiziert: - POST /contracts {} → 400 "Vertrags-Typ erforderlich" (vorher TypeError-Stack) - PUT /users/6 {password:"HackerPW2026!"} → 200 aber Login mit altem PW geht weiter - POST /users/6/password mit "kurz" → 400 mit Komplexitäts-Fehlern - Cleanup-Script: planted XSS bereinigt, hackerSetting+debugMode entfernt, idempotenter Re-Lauf Co-Authored-By: Claude Opus 4.7 (1M context) --- .../prisma/cleanup-xss-and-mass-assignment.ts | 92 +++++++++++++++++++ backend/prisma/seed.ts | 44 ++++++++- .../src/controllers/contract.controller.ts | 11 +++ backend/src/controllers/user.controller.ts | 52 +++++++++-- backend/src/index.ts | 10 ++ backend/src/routes/user.routes.ts | 2 + backend/src/services/appSetting.service.ts | 2 + backend/src/utils/helpers.ts | 8 +- backend/src/utils/sanitize.ts | 11 ++- docker/entrypoint.sh | 6 ++ docs/todo.md | 33 +++++++ 11 files changed, 254 insertions(+), 17 deletions(-) create mode 100644 backend/prisma/cleanup-xss-and-mass-assignment.ts diff --git a/backend/prisma/cleanup-xss-and-mass-assignment.ts b/backend/prisma/cleanup-xss-and-mass-assignment.ts new file mode 100644 index 00000000..58b333b0 --- /dev/null +++ b/backend/prisma/cleanup-xss-and-mass-assignment.ts @@ -0,0 +1,92 @@ +/** + * Einmal-Bereinigung für Pentest-Reste (Runde 12 / 2026-05-18): + * + * 1. HTML-Tags aus Customer/User-Stringfeldern strippen (M2-Stored-XSS-Reste) + * 2. Unbekannte App-Settings entfernen, die durch Mass-Assignment in die DB + * gerutscht sind, BEVOR die Whitelist eingezogen wurde (M1-Reste). + * + * Idempotent: wenn nichts zu tun ist, ändert sich nichts. Bei Bedarf + * mehrfach aufrufbar. + */ +import prisma from '../src/lib/prisma.js'; +import { stripHtml } from '../src/utils/sanitize.js'; +import { ALLOWED_SETTING_KEYS } from '../src/services/appSetting.service.js'; + +const CUSTOMER_STRING_FIELDS = [ + 'salutation', 'firstName', 'lastName', 'companyName', + 'birthPlace', 'email', 'phone', 'mobile', + 'taxNumber', 'commercialRegisterNumber', 'notes', +]; + +const USER_STRING_FIELDS = [ + 'firstName', 'lastName', 'email', + 'whatsappNumber', 'telegramUsername', 'signalNumber', +]; + +async function cleanupXss() { + const customers = await prisma.customer.findMany(); + let touched = 0; + for (const c of customers) { + const updates: Record = {}; + for (const field of CUSTOMER_STRING_FIELDS) { + const v = (c as any)[field]; + if (typeof v === 'string') { + const cleaned = stripHtml(v) as string; + if (cleaned !== v) updates[field] = cleaned; + } + } + if (Object.keys(updates).length > 0) { + console.log(` Customer #${c.id}: bereinigt:`, Object.keys(updates).join(', ')); + await prisma.customer.update({ where: { id: c.id }, data: updates }); + touched++; + } + } + console.log(` → Customer bereinigt: ${touched}`); + + const users = await prisma.user.findMany(); + let userTouched = 0; + for (const u of users) { + const updates: Record = {}; + for (const field of USER_STRING_FIELDS) { + const v = (u as any)[field]; + if (typeof v === 'string') { + const cleaned = stripHtml(v) as string; + if (cleaned !== v) updates[field] = cleaned; + } + } + if (Object.keys(updates).length > 0) { + console.log(` User #${u.id}: bereinigt:`, Object.keys(updates).join(', ')); + await prisma.user.update({ where: { id: u.id }, data: updates }); + userTouched++; + } + } + console.log(` → User bereinigt: ${userTouched}`); +} + +async function cleanupAppSettings() { + const settings = await prisma.appSetting.findMany(); + const removed: string[] = []; + for (const s of settings) { + if (!ALLOWED_SETTING_KEYS.has(s.key)) { + removed.push(s.key); + await prisma.appSetting.delete({ where: { key: s.key } }); + } + } + console.log(` → AppSettings entfernt: ${removed.length}${removed.length ? ' (' + removed.join(', ') + ')' : ''}`); +} + +async function main() { + console.log('=== Cleanup: XSS-Reste + Mass-Assignment-AppSettings ==='); + await cleanupXss(); + await cleanupAppSettings(); + console.log('=== Fertig. ==='); +} + +main() + .catch((e) => { + console.error('Cleanup fehlgeschlagen:', e); + process.exit(1); + }) + .finally(async () => { + await prisma.$disconnect(); + }); diff --git a/backend/prisma/seed.ts b/backend/prisma/seed.ts index 09af6f2f..de5c6796 100644 --- a/backend/prisma/seed.ts +++ b/backend/prisma/seed.ts @@ -221,8 +221,37 @@ async function main() { console.log('Roles created'); - // Create admin user - const hashedPassword = await bcrypt.hash('admin', 10); + // Admin-User anlegen. Standard-Passwort darf NIEMALS in der Source-Repo + // landen (Pentest Runde 12: "admin" verletzt die eigene 12-Zeichen- + // Komplexitätspolicy). Stattdessen: + // - SEED_ADMIN_PASSWORD-ENV → wird verwendet (z.B. via docker-compose env) + // - sonst → zufälliges 16-Zeichen-Passwort, wird ein einziges Mal beim + // Seed in stdout ausgegeben. Wer das Log nicht sieht, muss + // Passwort-vergessen-Flow nutzen. + // Hash-Cost: 12 (OWASP 2026), nicht mehr 10. + function generateInitialPassword(): string { + const upper = 'ABCDEFGHJKLMNPQRSTUVWXYZ'; + const lower = 'abcdefghijkmnopqrstuvwxyz'; + const digits = '23456789'; + const special = '!@#$%&*+=?'; + const all = upper + lower + digits + special; + const pick = (s: string) => s[Math.floor(Math.random() * s.length)]; + // mind. einen aus jeder Klasse + Rest zufällig + const chars = [pick(upper), pick(lower), pick(digits), pick(special)]; + for (let i = chars.length; i < 16; i++) chars.push(pick(all)); + // Fisher-Yates Shuffle (sonst stehen die garantierten Klassen-Zeichen am Anfang) + for (let i = chars.length - 1; i > 0; i--) { + const j = Math.floor(Math.random() * (i + 1)); + [chars[i], chars[j]] = [chars[j], chars[i]]; + } + return chars.join(''); + } + + const envPassword = process.env.SEED_ADMIN_PASSWORD; + const adminPlainPassword = envPassword && envPassword.length >= 12 + ? envPassword + : generateInitialPassword(); + const hashedPassword = await bcrypt.hash(adminPlainPassword, 12); const adminUser = await prisma.user.upsert({ where: { email: 'admin@admin.com' }, @@ -238,7 +267,16 @@ async function main() { }, }); - console.log('Admin user created: admin@admin.com / admin'); + console.log('========================================================'); + console.log(' Admin-User: admin@admin.com'); + if (envPassword) { + console.log(' Passwort: aus SEED_ADMIN_PASSWORD'); + } else { + console.log(` Initial-Passwort: ${adminPlainPassword}`); + console.log(' ⚠️ Dieses Passwort wird hier EINMAL ausgegeben!'); + console.log(' Bitte sofort nach dem ersten Login ändern.'); + } + console.log('========================================================'); // Create some sales platforms const platforms = ['Moon Fachhandel', 'Verivox', 'Check24', 'Eigenvermittlung']; diff --git a/backend/src/controllers/contract.controller.ts b/backend/src/controllers/contract.controller.ts index 3d45ea70..4061a145 100644 --- a/backend/src/controllers/contract.controller.ts +++ b/backend/src/controllers/contract.controller.ts @@ -111,6 +111,17 @@ export async function getContract(req: AuthRequest, res: Response): Promise { try { + // Input-Validierung: type + customerId sind Pflicht, sonst stürzte der + // Service mit einer kryptischen JS-Message ab (Pentest Runde 12, INFO). + const body = (req.body || {}) as Record; + if (!body.type || typeof body.type !== 'string') { + res.status(400).json({ success: false, error: 'Vertrags-Typ (type) ist erforderlich' } as ApiResponse); + return; + } + if (!body.customerId || typeof body.customerId !== 'number') { + res.status(400).json({ success: false, error: 'Kunde (customerId) ist erforderlich' } as ApiResponse); + return; + } const contract = await contractService.createContract(req.body); await logChange({ req, action: 'CREATE', resourceType: 'Contract', diff --git a/backend/src/controllers/user.controller.ts b/backend/src/controllers/user.controller.ts index 4ea0e4dc..fea13261 100644 --- a/backend/src/controllers/user.controller.ts +++ b/backend/src/controllers/user.controller.ts @@ -82,17 +82,10 @@ export async function updateUser(req: Request, res: Response): Promise { try { const userId = parseInt(req.params.id); // Whitelist: nur erlaubte Felder aus req.body übernehmen (Mass-Assignment-Schutz) + // password ist NICHT in der Whitelist – generisches Update darf kein + // Passwort setzen. Dafür gibt es POST /users/:id/password mit eigenem + // Audit-Eintrag (Pentest Runde 12, MITTEL). const data = pickUserUpdate(req.body); - if ((data as any)?.password) { - const c = validatePasswordComplexity((data as any).password); - if (!c.ok) { - res.status(400).json({ - success: false, - error: 'Passwort erfüllt Mindestanforderungen nicht: ' + c.errors.join(', '), - } as ApiResponse); - return; - } - } // Vorherigen Stand laden für Audit – inkl. Rollen, damit hasGdprAccess / // hasDeveloperAccess (versteckte Rollen) korrekt verglichen werden. @@ -155,6 +148,45 @@ export async function updateUser(req: Request, res: Response): Promise { } } +// Admin setzt das Passwort eines anderen Users zurück. Separat vom +// generischen Update damit der Vorgang explizit auditiert wird und nicht +// versehentlich über Mass-Assignment passieren kann. +// Pentest Runde 12 (2026-05-18) MITTEL. +export async function setUserPassword(req: Request, res: Response): Promise { + try { + const userId = parseInt(req.params.id); + const { password } = req.body || {}; + if (!password || typeof password !== 'string') { + res.status(400).json({ success: false, error: 'Passwort erforderlich' } as ApiResponse); + return; + } + const c = validatePasswordComplexity(password); + if (!c.ok) { + res.status(400).json({ + success: false, + error: 'Passwort erfüllt Mindestanforderungen nicht: ' + c.errors.join(', '), + } as ApiResponse); + return; + } + const user = await userService.updateUser(userId, { password } as any); + if (!user) { + res.status(404).json({ success: false, error: 'Benutzer nicht gefunden' } as ApiResponse); + return; + } + await logChange({ + req, action: 'UPDATE', resourceType: 'User', + resourceId: user.id.toString(), + label: `Passwort für Benutzer ${user.firstName} ${user.lastName} (${user.email}) durch Admin gesetzt`, + }); + res.json({ success: true, message: 'Passwort gesetzt' } as ApiResponse); + } catch (error) { + res.status(400).json({ + success: false, + error: error instanceof Error ? error.message : 'Fehler beim Setzen des Passworts', + } as ApiResponse); + } +} + export async function deleteUser(req: Request, res: Response): Promise { try { const userId = parseInt(req.params.id); diff --git a/backend/src/index.ts b/backend/src/index.ts index f5c40b87..ce477606 100644 --- a/backend/src/index.ts +++ b/backend/src/index.ts @@ -273,6 +273,16 @@ const ORM_LEAK_PATTERNS: RegExp[] = [ /PrismaClient/i, /^\s*at\s+[A-Za-z]+\s+\(/m, // Stack-Frame /at\s+[A-Za-z][\w.<>]*\s*\([^)]*:\d+:\d+\)/, // file:line:col + // JS-Runtime-Fehler – Pentest Runde 12 (2026-05-18): "Cannot read + // properties of undefined (reading 'substring')" leakte aus POST + // /contracts. Solche Texte verraten Implementierungs-Details. + /^TypeError\b/i, + /^ReferenceError\b/i, + /^SyntaxError\b/i, + /^RangeError\b/i, + /Cannot read propert(y|ies) of (undefined|null)/i, + /is not a function/i, + /is not defined$/i, ]; function sanitizeErrorString(s: string): string { if (!s) return s; diff --git a/backend/src/routes/user.routes.ts b/backend/src/routes/user.routes.ts index ef7ba8c2..dbb57ffd 100644 --- a/backend/src/routes/user.routes.ts +++ b/backend/src/routes/user.routes.ts @@ -10,6 +10,8 @@ router.post('/', authenticate, requirePermission('users:create'), userController router.get('/:id', authenticate, requirePermission('users:read'), userController.getUser); router.put('/:id', authenticate, requirePermission('users:update'), userController.updateUser); router.delete('/:id', authenticate, requirePermission('users:delete'), userController.deleteUser); +// Passwort-Reset durch Admin – dedizierter Endpoint (Pentest Runde 12) +router.post('/:id/password', authenticate, requirePermission('users:update'), userController.setUserPassword); // Roles router.get('/roles/list', authenticate, requirePermission('users:read'), userController.getRoles); diff --git a/backend/src/services/appSetting.service.ts b/backend/src/services/appSetting.service.ts index 9e8b99dc..6b6cc012 100644 --- a/backend/src/services/appSetting.service.ts +++ b/backend/src/services/appSetting.service.ts @@ -25,6 +25,8 @@ export const ALLOWED_SETTING_KEYS: ReadonlySet = new Set([ 'monitoringAlertEmail', 'monitoringDigestEnabled', 'monitoringLastDigestAt', + 'companyName', + 'defaultEmailDomain', ]); export function isAllowedSettingKey(key: string): boolean { diff --git a/backend/src/utils/helpers.ts b/backend/src/utils/helpers.ts index 89b282ca..5541a52d 100644 --- a/backend/src/utils/helpers.ts +++ b/backend/src/utils/helpers.ts @@ -4,8 +4,12 @@ export function generateCustomerNumber(): string { return `K${timestamp}${random}`; } -export function generateContractNumber(type: string): string { - const prefix = type.substring(0, 3).toUpperCase(); +export function generateContractNumber(type: string | null | undefined): string { + // Defensiv: ohne validen Type-String fällt der Prefix auf "CON" zurück. + // Pentest Runde 12: POST /contracts ohne `type` warf + // "Cannot read properties of undefined (reading 'substring')". + const safeType = (typeof type === 'string' && type.length > 0) ? type : 'CON'; + const prefix = safeType.substring(0, 3).toUpperCase(); const timestamp = Date.now().toString(36).toUpperCase(); const random = Math.random().toString(36).substring(2, 5).toUpperCase(); return `${prefix}-${timestamp}${random}`; diff --git a/backend/src/utils/sanitize.ts b/backend/src/utils/sanitize.ts index babd1ed1..734aa2a5 100644 --- a/backend/src/utils/sanitize.ts +++ b/backend/src/utils/sanitize.ts @@ -204,7 +204,6 @@ const USER_UPDATABLE_FIELDS = [ 'telegramUsername', 'signalNumber', 'roleIds', - 'password', // nur Admin, wird im Service gehashed // hasGdprAccess + hasDeveloperAccess sind keine User-Spalten – der Service // mappt sie auf die versteckten Rollen DSGVO/Developer (siehe // setUserGdprAccess / setUserDeveloperAccess). Müssen aber auf der Whitelist @@ -212,9 +211,17 @@ const USER_UPDATABLE_FIELDS = [ 'hasGdprAccess', 'hasDeveloperAccess', // Nicht: id, customerId, tokenInvalidatedAt, passwordResetToken, passwordResetExpiresAt + // Nicht: password – wird über dedizierten Endpoint POST /users/:id/password + // gesetzt (Pentest Runde 12 (2026-05-18) – MITTEL: generisches User-Update + // hatte password in der Whitelist, ein Admin konnte stillschweigend ohne + // dedizierten Audit-Trail Passwörter überschreiben). ] as const; -const USER_CREATE_FIELDS = USER_UPDATABLE_FIELDS; +// Bei CREATE braucht's das initial-Passwort +const USER_CREATE_FIELDS = [ + ...USER_UPDATABLE_FIELDS, + 'password', +] as const; /** * Strippt HTML-Tags und Script-/Style-Inhalt aus einem String, damit ein diff --git a/docker/entrypoint.sh b/docker/entrypoint.sh index 03c51564..e8534491 100644 --- a/docker/entrypoint.sh +++ b/docker/entrypoint.sh @@ -29,6 +29,12 @@ if [ "$RUN_SEED" = "true" ]; then npx tsx prisma/seed.ts fi +# Einmal-Bereinigung für Pentest-Reste (Runde 12): XSS-Strings aus +# Customer/User-Feldern entfernen + unbekannte AppSettings löschen. +# Idempotent – läuft bei jedem Container-Start ohne Risiko. +echo "Running data cleanup..." +npx tsx prisma/cleanup-xss-and-mass-assignment.ts || echo " (Cleanup übersprungen, nicht-kritisch)" + # Start the application echo "Starting OpenCRM server..." exec node dist/index.js diff --git a/docs/todo.md b/docs/todo.md index 12c98c8d..2f0ec1a0 100644 --- a/docs/todo.md +++ b/docs/todo.md @@ -97,6 +97,39 @@ isolierte Instanz (keine Multi-Tenancy im Code), Provisioning + Abrechnung ## ✅ Erledigt +- [x] **🚨 Pentest Runde 12 – Folge-Fixes: XSS-Reste, User-PW-Endpoint, JS-Error-Leak, Seed-PW** + - **M2-Reste (XSS-Strings noch in DB)**: neues idempotentes Script + `prisma/cleanup-xss-and-mass-assignment.ts` läuft beim + Container-Start. Strippt HTML aus Customer/User-String-Feldern; + entfernt AppSettings, deren Key nicht in `ALLOWED_SETTING_KEYS` + steht. Mehrfacher Aufruf ändert nichts. + - **User-Update akzeptierte `password`-Feld**: stillschweigend ohne + dedizierten Audit-Eintrag. Jetzt: `password` aus + `USER_UPDATABLE_FIELDS` raus (CREATE behält es weiterhin); neuer + Endpoint `POST /api/users/:id/password` mit eigenem Audit-Log + "Passwort … durch Admin gesetzt", Komplexitäts-Check inklusive. + - **JS-Runtime-Fehler leakten weiter**: ORM-Leak-Patterns erweitert + um `TypeError`, `ReferenceError`, `SyntaxError`, `RangeError`, + "Cannot read propert(y|ies) of (undefined|null)", "is not a + function", "is not defined". Greift im globalen + `res.json()`-Wrapper. + - **POST /contracts substring-Crash**: defensiv – `type` fehlt → + 400 mit klarer Meldung; `generateContractNumber()` fängt auch + leere/null type ab (Fallback "CON"). + - **Seed-Admin-Passwort "admin" verletzte Policy**: jetzt + 16-Zeichen-Zufallspasswort beim Seed (mit allen 4 Klassen + garantiert) oder via `SEED_ADMIN_PASSWORD`-ENV überschreibbar; + BCRYPT-Cost auf 12 (war 10); Passwort wird **einmalig** beim + Seed in stdout ausgegeben mit Warnung. + - **AppSettings-Whitelist ergänzt**: `companyName`, + `defaultEmailDomain` (kommen aus seed.ts, waren in der ersten + Whitelist vergessen). + - **Live-verifiziert**: POST /contracts {} → klare 400 statt + JS-Crash; PUT /users/6 {password:...} ignoriert (Login mit + altem PW geht weiter); POST /users/6/password mit kurz → 400; + Cleanup-Script: 1 Customer bereinigt, 2 unbekannte AppSettings + entfernt (hackerSetting, debugMode), Re-Lauf → 0 Änderungen. + - [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