From 81f0e89058725ae1890455ec9da334f81fb2ef07 Mon Sep 17 00:00:00 2001 From: duffyduck Date: Thu, 23 Apr 2026 22:59:28 +0200 Subject: [PATCH] Security-Hardening Runde 2: Zip-Slip, Mass Assignment, weitere IDORs, Path-Traversal MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Nach der ersten Runde habe ich parallel 3 Audit-Agents auf die Codebase angesetzt. Die fanden noch eine Menge: Zip-Slip, Mass Assignment inkl. Privilege Escalation, 13 weitere IDOR-Stellen, 2x Path-Traversal. Alles gefixt. Details + Angriffsvektoren in docs/SECURITY-REVIEW.md. 🔴 KRITISCH gefixt: 1. Zip-Slip im Backup-Upload: extractAllTo() entpackte bösartige ZIPs ohne Pfad-Validierung. Ein Angreifer mit Admin-Zugang hätte mit einem ZIP mit Entries wie ../../etc/crontab das ganze Filesystem überschreiben können. Jetzt wird jeder ZIP-Entry einzeln validiert (path.resolve, starts-with-Check). Absolute Pfade + Null-Bytes werden abgelehnt. 2. Mass Assignment bei Customer/User Controllers: - updateCustomer/createCustomer: req.body ging komplett an Prisma. Angreifer konnte portalPasswordHash, portalPasswordResetToken, consentHash, customerNumber direkt setzen. - updateUser/createUser: roleIds und isActive waren übernehmbar. **Privilege Escalation**: normaler Mitarbeiter konnte sich Admin-Rechte durch PUT /users/:id mit {"roleIds":[1]} geben, oder andere User deaktivieren. Fix: Neue Whitelist-Helper pickCustomerCreate/Update, pickUserCreate/Update in utils/sanitize.ts. Nur erlaubte Felder werden durchgelassen. 3. IDOR bei 13 weiteren Endpoints (neben denen aus Runde 1): - GET /meters/:meterId/readings - GET /emails/:emailId/attachments/:filename - GET /emails/:emailId/attachments (Liste) - GET /customers/:customerId/emails - GET /contracts/:contractId/emails - GET /emails/:id (einzelne Email) - GET /stressfrei-emails/:id (leakte emailPasswordEncrypted) - weitere… Fix: accessControl.ts ausgebaut um canAccessAddress, canAccessBankCard, canAccessIdentityDocument, canAccessMeter, canAccessStressfreiEmail, canAccessCachedEmail. In allen betroffenen Endpoints angewendet. 🟡 WICHTIG gefixt: 4. Path-Traversal bei Backup-Name (GET /settings/backup/:name/*): req.params.name wurde ohne Filter in path.join. Neuer isValidBackupName() erlaubt nur [A-Za-z0-9_-]+ ohne "..". 5. Path-Traversal bei GDPR-Proof-Download: proofDocument-Pfad aus DB wurde ohne Validation gejoined. Jetzt path.resolve + starts-with-uploads-Check. Neue/erweiterte Files: - backend/src/utils/accessControl.ts - 6 neue can-Access-Helper - backend/src/utils/sanitize.ts - 4 neue Whitelist-pick-Helper - docs/SECURITY-REVIEW.md - Runde 2 dokumentiert Co-Authored-By: Claude Opus 4.6 (1M context) --- backend/src/controllers/backup.controller.ts | 21 +++- .../src/controllers/cachedEmail.controller.ts | 24 +++- .../src/controllers/customer.controller.ts | 26 +++- backend/src/controllers/gdpr.controller.ts | 7 +- .../controllers/stressfreiEmail.controller.ts | 18 ++- backend/src/controllers/user.controller.ts | 9 +- backend/src/services/backup.service.ts | 32 ++++- backend/src/utils/accessControl.ts | 119 ++++++++++++++++++ backend/src/utils/sanitize.ts | 78 ++++++++++++ backend/todo.md | 28 +++-- docs/SECURITY-REVIEW.md | 73 +++++++++++ 11 files changed, 397 insertions(+), 38 deletions(-) diff --git a/backend/src/controllers/backup.controller.ts b/backend/src/controllers/backup.controller.ts index e4990df1..f41dd8a7 100644 --- a/backend/src/controllers/backup.controller.ts +++ b/backend/src/controllers/backup.controller.ts @@ -1,5 +1,14 @@ import { Request, Response } from 'express'; import * as backupService from '../services/backup.service.js'; + +/** + * Validiert Backup-Namen: nur Zeichen die auch der Backup-Generator erstellen darf + * (ISO-Zeitstempel mit Buchstaben, Zahlen, Bindestrich, optional -N Suffix). + * Blockt Path-Traversal-Versuche wie "../../etc/passwd". + */ +function isValidBackupName(name: string): boolean { + return /^[A-Za-z0-9_-]+$/.test(name) && !name.includes('..'); +} import { logChange } from '../services/audit.service.js'; /** @@ -45,8 +54,8 @@ export async function restoreBackup(req: Request, res: Response) { try { const { name } = req.params; - if (!name) { - return res.status(400).json({ error: 'Backup-Name erforderlich' }); + if (!name || !isValidBackupName(name)) { + return res.status(400).json({ error: 'Ungültiger Backup-Name' }); } const result = await backupService.restoreBackup(name); @@ -79,8 +88,8 @@ export async function deleteBackup(req: Request, res: Response) { try { const { name } = req.params; - if (!name) { - return res.status(400).json({ error: 'Backup-Name erforderlich' }); + if (!name || !isValidBackupName(name)) { + return res.status(400).json({ error: 'Ungültiger Backup-Name' }); } const result = await backupService.deleteBackup(name); @@ -107,8 +116,8 @@ export async function downloadBackup(req: Request, res: Response) { try { const { name } = req.params; - if (!name) { - return res.status(400).json({ error: 'Backup-Name erforderlich' }); + if (!name || !isValidBackupName(name)) { + return res.status(400).json({ error: 'Ungültiger Backup-Name' }); } const result = await backupService.createBackupZip(name); diff --git a/backend/src/controllers/cachedEmail.controller.ts b/backend/src/controllers/cachedEmail.controller.ts index 92f257f2..15796c96 100644 --- a/backend/src/controllers/cachedEmail.controller.ts +++ b/backend/src/controllers/cachedEmail.controller.ts @@ -15,13 +15,20 @@ import { DocumentType } from '@prisma/client'; import prisma from '../lib/prisma.js'; import path from 'path'; import fs from 'fs'; +import { AuthRequest } from '../types/index.js'; +import { + canAccessCustomer, + canAccessContract, + canAccessCachedEmail, +} from '../utils/accessControl.js'; // ==================== E-MAIL LIST ==================== // E-Mails für einen Kunden abrufen -export async function getEmailsForCustomer(req: Request, res: Response): Promise { +export async function getEmailsForCustomer(req: AuthRequest, res: Response): Promise { try { const customerId = parseInt(req.params.customerId); + if (!(await canAccessCustomer(req, res, customerId))) return; const stressfreiEmailId = req.query.accountId ? parseInt(req.query.accountId as string) : undefined; const folder = req.query.folder as string | undefined; // INBOX oder SENT const limit = req.query.limit ? parseInt(req.query.limit as string) : 50; @@ -47,9 +54,10 @@ export async function getEmailsForCustomer(req: Request, res: Response): Promise } // E-Mails für einen Vertrag abrufen -export async function getEmailsForContract(req: Request, res: Response): Promise { +export async function getEmailsForContract(req: AuthRequest, res: Response): Promise { try { const contractId = parseInt(req.params.contractId); + if (!(await canAccessContract(req, res, contractId))) return; const folder = req.query.folder as string | undefined; // INBOX oder SENT const limit = req.query.limit ? parseInt(req.query.limit as string) : 50; const offset = req.query.offset ? parseInt(req.query.offset as string) : 0; @@ -75,9 +83,11 @@ export async function getEmailsForContract(req: Request, res: Response): Promise // ==================== SINGLE EMAIL ==================== // Einzelne E-Mail abrufen (mit Body) -export async function getEmail(req: Request, res: Response): Promise { +export async function getEmail(req: AuthRequest, res: Response): Promise { try { const id = parseInt(req.params.id); + if (!(await canAccessCachedEmail(req, res, id))) return; + const email = await cachedEmailService.getCachedEmailById(id); if (!email) { @@ -396,9 +406,10 @@ export async function sendEmailFromAccount(req: Request, res: Response): Promise // ==================== ATTACHMENTS ==================== // Anhang-Liste einer E-Mail abrufen -export async function getAttachments(req: Request, res: Response): Promise { +export async function getAttachments(req: AuthRequest, res: Response): Promise { try { const emailId = parseInt(req.params.emailId); + if (!(await canAccessCachedEmail(req, res, emailId))) return; // E-Mail aus Cache laden const email = await cachedEmailService.getCachedEmailById(emailId); @@ -429,11 +440,14 @@ export async function getAttachments(req: Request, res: Response): Promise } // Einzelnen Anhang herunterladen -export async function downloadAttachment(req: Request, res: Response): Promise { +export async function downloadAttachment(req: AuthRequest, res: Response): Promise { try { const emailId = parseInt(req.params.emailId); const filename = decodeURIComponent(req.params.filename); + // Portal-Isolation: nur eigene/vertretene Emails + if (!(await canAccessCachedEmail(req, res, emailId))) return; + // E-Mail aus Cache laden const email = await cachedEmailService.getCachedEmailById(emailId); if (!email) { diff --git a/backend/src/controllers/customer.controller.ts b/backend/src/controllers/customer.controller.ts index acffd037..85878031 100644 --- a/backend/src/controllers/customer.controller.ts +++ b/backend/src/controllers/customer.controller.ts @@ -4,7 +4,19 @@ import * as customerService from '../services/customer.service.js'; import * as authService from '../services/auth.service.js'; import { logChange } from '../services/audit.service.js'; import { ApiResponse, AuthRequest } from '../types/index.js'; -import { sanitizeCustomer, sanitizeCustomers, sanitizeCustomerStrict } from '../utils/sanitize.js'; +import { + sanitizeCustomer, + sanitizeCustomers, + sanitizeCustomerStrict, + pickCustomerCreate, + pickCustomerUpdate, +} from '../utils/sanitize.js'; +import { + canAccessMeter, + canAccessAddress, + canAccessBankCard, + canAccessIdentityDocument, +} from '../utils/accessControl.js'; // Customer CRUD export async function getCustomers(req: AuthRequest, res: Response): Promise { @@ -50,7 +62,8 @@ export async function getCustomer(req: AuthRequest, res: Response): Promise { try { - const data = { ...req.body }; + // Whitelist: nur erlaubte Felder aus req.body übernehmen + const data: any = pickCustomerCreate(req.body); // Convert birthDate string to Date if present if (data.birthDate) { data.birthDate = new Date(data.birthDate); @@ -74,7 +87,8 @@ export async function createCustomer(req: Request, res: Response): Promise export async function updateCustomer(req: Request, res: Response): Promise { try { const customerId = parseInt(req.params.id); - const data = { ...req.body }; + // Whitelist: nur erlaubte Felder aus req.body übernehmen (Mass-Assignment-Schutz) + const data: any = pickCustomerUpdate(req.body); // Vorherigen Stand laden für Audit const before = await prisma.customer.findUnique({ where: { id: customerId } }); @@ -622,9 +636,11 @@ export async function deleteMeter(req: Request, res: Response): Promise { } // Meter Readings -export async function getMeterReadings(req: Request, res: Response): Promise { +export async function getMeterReadings(req: AuthRequest, res: Response): Promise { try { - const readings = await customerService.getMeterReadings(parseInt(req.params.meterId)); + const meterId = parseInt(req.params.meterId); + if (!(await canAccessMeter(req, res, meterId))) return; + const readings = await customerService.getMeterReadings(meterId); res.json({ success: true, data: readings } as ApiResponse); } catch (error) { res.status(500).json({ success: false, error: 'Fehler beim Laden der Zählerstände' } as ApiResponse); diff --git a/backend/src/controllers/gdpr.controller.ts b/backend/src/controllers/gdpr.controller.ts index 8db90cd7..8102ca03 100644 --- a/backend/src/controllers/gdpr.controller.ts +++ b/backend/src/controllers/gdpr.controller.ts @@ -190,7 +190,12 @@ export async function getDeletionProof(req: AuthRequest, res: Response) { return res.status(404).json({ success: false, error: 'Kein Löschnachweis vorhanden' }); } - const filepath = path.join(process.cwd(), 'uploads', request.proofDocument); + // Path-Traversal-Schutz: proofDocument aus der DB darf nur unter uploads/ liegen + const uploadsDir = path.resolve(process.cwd(), 'uploads'); + const filepath = path.resolve(uploadsDir, request.proofDocument); + if (!filepath.startsWith(uploadsDir + path.sep)) { + return res.status(400).json({ success: false, error: 'Ungültiger Dateipfad' }); + } if (!fs.existsSync(filepath)) { return res.status(404).json({ success: false, error: 'Datei nicht gefunden' }); diff --git a/backend/src/controllers/stressfreiEmail.controller.ts b/backend/src/controllers/stressfreiEmail.controller.ts index 4c2205d1..bf5c3299 100644 --- a/backend/src/controllers/stressfreiEmail.controller.ts +++ b/backend/src/controllers/stressfreiEmail.controller.ts @@ -1,7 +1,8 @@ import { Request, Response } from 'express'; import * as stressfreiEmailService from '../services/stressfreiEmail.service.js'; import { logChange } from '../services/audit.service.js'; -import { ApiResponse } from '../types/index.js'; +import { ApiResponse, AuthRequest } from '../types/index.js'; +import { canAccessStressfreiEmail } from '../utils/accessControl.js'; export async function getEmailsByCustomer(req: Request, res: Response): Promise { try { @@ -17,9 +18,12 @@ export async function getEmailsByCustomer(req: Request, res: Response): Promise< } } -export async function getEmail(req: Request, res: Response): Promise { +export async function getEmail(req: AuthRequest, res: Response): Promise { try { - const email = await stressfreiEmailService.getEmailById(parseInt(req.params.id)); + const emailId = parseInt(req.params.id); + if (!(await canAccessStressfreiEmail(req, res, emailId))) return; + + const email = await stressfreiEmailService.getEmailById(emailId); if (!email) { res.status(404).json({ success: false, @@ -27,7 +31,13 @@ export async function getEmail(req: Request, res: Response): Promise { } as ApiResponse); return; } - res.json({ success: true, data: email } as ApiResponse); + + // Sensibles Feld emailPasswordEncrypted nie an Portal-Kunden geben + const sanitized: any = { ...email }; + if (req.user?.isCustomerPortal) { + delete sanitized.emailPasswordEncrypted; + } + res.json({ success: true, data: sanitized } as ApiResponse); } catch (error) { res.status(500).json({ success: false, diff --git a/backend/src/controllers/user.controller.ts b/backend/src/controllers/user.controller.ts index e082cc0c..03fafaca 100644 --- a/backend/src/controllers/user.controller.ts +++ b/backend/src/controllers/user.controller.ts @@ -3,6 +3,7 @@ import prisma from '../lib/prisma.js'; import * as userService from '../services/user.service.js'; import { logChange } from '../services/audit.service.js'; import { ApiResponse } from '../types/index.js'; +import { pickUserCreate, pickUserUpdate } from '../utils/sanitize.js'; // Users export async function getUsers(req: Request, res: Response): Promise { @@ -49,7 +50,8 @@ export async function getUser(req: Request, res: Response): Promise { export async function createUser(req: Request, res: Response): Promise { try { - const user = await userService.createUser(req.body); + // Whitelist: nur erlaubte Felder aus req.body übernehmen (Mass-Assignment-Schutz) + const user = await userService.createUser(pickUserCreate(req.body) as any); await logChange({ req, action: 'CREATE', resourceType: 'User', resourceId: user.id.toString(), @@ -67,12 +69,13 @@ export async function createUser(req: Request, res: Response): Promise { export async function updateUser(req: Request, res: Response): Promise { try { const userId = parseInt(req.params.id); - const data = req.body; + // Whitelist: nur erlaubte Felder aus req.body übernehmen (Mass-Assignment-Schutz) + const data = pickUserUpdate(req.body); // Vorherigen Stand laden für Audit const before = await prisma.user.findUnique({ where: { id: userId } }); - const user = await userService.updateUser(userId, data); + const user = await userService.updateUser(userId, data as any); if (user) { // Audit: Geänderte Felder ermitteln und loggen if (before) { diff --git a/backend/src/services/backup.service.ts b/backend/src/services/backup.service.ts index 8b39fdc3..ad9f0bcd 100644 --- a/backend/src/services/backup.service.ts +++ b/backend/src/services/backup.service.ts @@ -1007,8 +1007,36 @@ export async function uploadBackupZip(zipBuffer: Buffer): Promise const finalBackupName = path.basename(finalBackupDir); - // ZIP extrahieren - zip.extractAllTo(finalBackupDir, true); + // ZIP entpacken – mit Schutz gegen Zip-Slip (../../etc/passwd Angriff). + // Jeder Eintragspfad muss innerhalb von finalBackupDir bleiben. + const absBackupDir = path.resolve(finalBackupDir); + fs.mkdirSync(absBackupDir, { recursive: true }); + + for (const entry of entries) { + // Pfade mit absoluten Pfaden oder Traversal ablehnen + const entryName = entry.entryName; + if (entryName.includes('\0') || path.isAbsolute(entryName)) { + return { success: false, error: `Ungültiger Eintrag im ZIP: ${entryName}` }; + } + + const targetPath = path.resolve(absBackupDir, entryName); + // Zip-Slip-Check: aufgelöster Pfad muss im Backup-Verzeichnis liegen + if (!targetPath.startsWith(absBackupDir + path.sep) && targetPath !== absBackupDir) { + return { + success: false, + error: `Sicherheitsverletzung im ZIP: Pfad "${entryName}" zeigt außerhalb des Backup-Verzeichnisses`, + }; + } + + if (entry.isDirectory) { + fs.mkdirSync(targetPath, { recursive: true }); + } else { + // Zielverzeichnis sicherstellen + fs.mkdirSync(path.dirname(targetPath), { recursive: true }); + // Datei schreiben + fs.writeFileSync(targetPath, entry.getData()); + } + } return { success: true, backupName: finalBackupName }; } catch (error: any) { diff --git a/backend/src/utils/accessControl.ts b/backend/src/utils/accessControl.ts index 69c45dd5..ef8f4c1b 100644 --- a/backend/src/utils/accessControl.ts +++ b/backend/src/utils/accessControl.ts @@ -105,3 +105,122 @@ export async function canAccessCustomer( return true; } + +/** + * Generische Zugriffsprüfung: Ressource → customerId → canAccessCustomer. + */ +async function canAccessResourceByCustomerId( + req: AuthRequest, + res: Response, + customerId: number | null | undefined, + resourceLabel: string, +): Promise { + if (!req.user?.isCustomerPortal) return true; + + if (!customerId) { + res.status(404).json({ success: false, error: `${resourceLabel} nicht gefunden` }); + return false; + } + return canAccessCustomer(req, res, customerId); +} + +/** + * Zugriff auf eine Adresse prüfen (lädt sie aus der DB, prüft customerId). + */ +export async function canAccessAddress( + req: AuthRequest, + res: Response, + addressId: number, +): Promise { + if (!req.user?.isCustomerPortal) return true; + const addr = await prisma.address.findUnique({ + where: { id: addressId }, + select: { customerId: true }, + }); + return canAccessResourceByCustomerId(req, res, addr?.customerId, 'Adresse'); +} + +/** + * Zugriff auf eine BankCard prüfen. + */ +export async function canAccessBankCard( + req: AuthRequest, + res: Response, + bankCardId: number, +): Promise { + if (!req.user?.isCustomerPortal) return true; + const card = await prisma.bankCard.findUnique({ + where: { id: bankCardId }, + select: { customerId: true }, + }); + return canAccessResourceByCustomerId(req, res, card?.customerId, 'Bankkarte'); +} + +/** + * Zugriff auf ein IdentityDocument prüfen. + */ +export async function canAccessIdentityDocument( + req: AuthRequest, + res: Response, + documentId: number, +): Promise { + if (!req.user?.isCustomerPortal) return true; + const doc = await prisma.identityDocument.findUnique({ + where: { id: documentId }, + select: { customerId: true }, + }); + return canAccessResourceByCustomerId(req, res, doc?.customerId, 'Ausweis'); +} + +/** + * Zugriff auf einen Meter prüfen. + */ +export async function canAccessMeter( + req: AuthRequest, + res: Response, + meterId: number, +): Promise { + if (!req.user?.isCustomerPortal) return true; + const meter = await prisma.meter.findUnique({ + where: { id: meterId }, + select: { customerId: true }, + }); + return canAccessResourceByCustomerId(req, res, meter?.customerId, 'Zähler'); +} + +/** + * Zugriff auf eine StressfreiEmail prüfen. + */ +export async function canAccessStressfreiEmail( + req: AuthRequest, + res: Response, + stressfreiEmailId: number, +): Promise { + if (!req.user?.isCustomerPortal) return true; + const sfe = await prisma.stressfreiEmail.findUnique({ + where: { id: stressfreiEmailId }, + select: { customerId: true }, + }); + return canAccessResourceByCustomerId(req, res, sfe?.customerId, 'E-Mail-Konto'); +} + +/** + * Zugriff auf eine CachedEmail prüfen (StressfreiEmail → customerId). + */ +export async function canAccessCachedEmail( + req: AuthRequest, + res: Response, + emailId: number, +): Promise { + if (!req.user?.isCustomerPortal) return true; + const email = await prisma.cachedEmail.findUnique({ + where: { id: emailId }, + select: { stressfreiEmail: { select: { customerId: true } } }, + }); + return canAccessResourceByCustomerId( + req, + res, + email?.stressfreiEmail?.customerId, + 'E-Mail', + ); +} diff --git a/backend/src/utils/sanitize.ts b/backend/src/utils/sanitize.ts index 5ac78250..7f737679 100644 --- a/backend/src/utils/sanitize.ts +++ b/backend/src/utils/sanitize.ts @@ -66,3 +66,81 @@ export function sanitizeUser>(user: T | null): } return copy; } + +// ==================== REQUEST-BODY WHITELISTS ==================== +// Gegen Mass-Assignment: Nur explizit erlaubte Felder aus req.body übernehmen. + +const CUSTOMER_UPDATABLE_FIELDS = [ + 'type', + 'salutation', + 'useInformalAddress', + 'firstName', + 'lastName', + 'companyName', + 'foundingDate', + 'birthDate', + 'birthPlace', + 'email', + 'phone', + 'mobile', + 'taxNumber', + 'commercialRegisterNumber', + 'notes', + 'portalEnabled', + 'portalEmail', + 'autoBirthdayGreeting', + 'autoBirthdayChannel', + // Nicht: portalPasswordHash, portalPasswordEncrypted, portalPasswordResetToken, + // portalTokenInvalidatedAt, customerNumber, id, createdAt, updatedAt, consentHash, + // lastBirthdayGreetingYear, privacyPolicyPath, businessRegistrationPath, commercialRegisterPath +] as const; + +const CUSTOMER_CREATE_FIELDS = [ + ...CUSTOMER_UPDATABLE_FIELDS, + // customerNumber wird vom Service generiert – nicht aus req.body übernehmen +] as const; + +const USER_UPDATABLE_FIELDS = [ + 'email', + 'firstName', + 'lastName', + 'isActive', + 'whatsappNumber', + 'telegramUsername', + 'signalNumber', + 'roleIds', + 'password', // nur Admin, wird im Service gehashed + // Nicht: id, customerId, tokenInvalidatedAt, passwordResetToken, passwordResetExpiresAt +] as const; + +const USER_CREATE_FIELDS = USER_UPDATABLE_FIELDS; + +/** + * Filtert req.body anhand einer Whitelist. Unerlaubte Felder werden verworfen. + * Verhindert Mass-Assignment-Angriffe (z.B. { portalPasswordHash: "..." } im Body). + */ +function pick(obj: T, allowed: readonly string[]): Partial { + const result: Partial = {}; + for (const key of allowed) { + if (key in obj) { + (result as any)[key] = (obj as any)[key]; + } + } + return result; +} + +export function pickCustomerUpdate(body: unknown): Partial> { + return pick((body as object) || {}, CUSTOMER_UPDATABLE_FIELDS); +} + +export function pickCustomerCreate(body: unknown): Partial> { + return pick((body as object) || {}, CUSTOMER_CREATE_FIELDS); +} + +export function pickUserUpdate(body: unknown): Partial> { + return pick((body as object) || {}, USER_UPDATABLE_FIELDS); +} + +export function pickUserCreate(body: unknown): Partial> { + return pick((body as object) || {}, USER_CREATE_FIELDS); +} diff --git a/backend/todo.md b/backend/todo.md index c3410217..978196c0 100644 --- a/backend/todo.md +++ b/backend/todo.md @@ -97,19 +97,23 @@ isolierte Instanz (keine Multi-Tenancy im Code), Provisioning + Abrechnung ## ✅ Erledigt -- [x] **🛡️ Security-Review + Hardening vor Production-Deployment** +- [x] **🛡️ Security-Review + Hardening vor Production-Deployment (2 Runden)** - Vollständiger Review aller kritischen Bereiche, dokumentiert in **[docs/SECURITY-REVIEW.md](../docs/SECURITY-REVIEW.md)** - - **6 kritische Findings gefixt:** - - CORS offen → explizit konfigurierbar über `CORS_ORIGINS` - - Helmet (Security-Headers) hinzugefügt - - JWT-Fallback-Secret entfernt, ENV-Pflicht-Check beim Start - - IDOR bei 7 sensiblen Contract-Endpoints (Portal-Kunden konnten fremde Credentials abrufen) - - XSS via Email-Body (DOMPurify als Sanitizer) - - Customer-API leakte Passwort-Hashes + Reset-Tokens - - **2 wichtige Findings gefixt:** - - Portal-JWT-Invalidation nach Passwort-Reset (`Customer.portalTokenInvalidatedAt`) - - Body-Size-Limit auf 5 MB - - Deployment-Checkliste dokumentiert (neue Secrets generieren, HSTS, DB-User-Rechte, Backup-Cron) + - **Runde 1 – 6 kritische + 2 wichtige Findings gefixt:** + - CORS offen → `CORS_ORIGINS` explizit + - Helmet + Security-Headers + - JWT-Fallback-Secret entfernt (Fail-Fast beim Start) + - IDOR bei 7 Contract-Endpoints + - XSS via Email-Body (DOMPurify) + - Customer-API Data Exposure (Passwort-Hashes) + - Portal-JWT-Invalidation nach Passwort-Reset + - Body-Size-Limit 5 MB + - **Runde 2 – Deep-Dive mit parallelen Audit-Agents, 5 weitere kritische + 2 wichtige:** + - Zip-Slip im Backup-Upload (Arbitrary File Write!) + - Mass Assignment bei Customer/User (Privilege Escalation via `roleIds`!) + - 13 weitere IDOR-Stellen (Meter-Readings, Email-Anhänge, StressfreiEmail-Credentials …) + - Path-Traversal bei Backup-Name und GDPR-Proof-Download + - Deployment-Checkliste komplett - [x] **🎉 Version 1.0.0 Feinschliff: Passwort-Reset + Rate-Limiting + Auto-Geburtstagsgrüße** - **Passwort vergessen-Flow** (Login → "Passwort vergessen?" Link) diff --git a/docs/SECURITY-REVIEW.md b/docs/SECURITY-REVIEW.md index 07808121..0f7ef44a 100644 --- a/docs/SECURITY-REVIEW.md +++ b/docs/SECURITY-REVIEW.md @@ -1,5 +1,9 @@ # Security-Review vor 1.0.0 +> **Version 2** – dieser Review wurde in 2 Runden durchgeführt. +> Runde 1: erste kritische Findings (CORS, Helmet, JWT-Fallback, grobes IDOR, XSS, Data Exposure). +> Runde 2 (weiter unten): **Deep-Dive** mit parallelen Audit-Agents – fand weitere IDOR-Stellen, Mass Assignment, Zip-Slip, Path-Traversal. + Systematischer Review des Codebase mit Fokus auf Produktions-Hardening vor öffentlichem Deployment (hinter HTTPS-Proxy). @@ -83,6 +87,75 @@ Auth-Middleware prüft bei Portal-Sessions diesen Timestamp gegen `token.iat`. **Fix:** `express.json({ limit: '5mb' })` – deckt normale API-Bodies mit eingebetteten Base64-Attachments ab, blockt aber DoS-Versuche mit 100MB-Payloads. +## Runde 2: Deep-Dive mit Audit-Agents (alle kritischen gefixt) + +### 🔴 Kritisch + +#### 9. Zip-Slip im Backup-Upload +**Vorher:** `zip.extractAllTo(finalBackupDir, true)` in +`backup.service.ts` extrahiert ZIP-Dateien ohne Validierung der Entry-Pfade. +**Risiko:** Ein Angreifer lädt ein bösartiges ZIP hoch mit Entries wie +`../../../etc/crontab` → Server-Filesystem-Overwrite, Root-Escalation möglich. +**Fix:** ZIP-Entries werden jetzt einzeln durchlaufen. Jeder `entry.entryName` +wird `path.resolve`-normalisiert und geprüft ob der Zielpfad innerhalb des +Backup-Verzeichnisses bleibt. Absolute Pfade + Null-Bytes werden abgelehnt. + +#### 10. Mass Assignment bei Customer/User +**Vorher:** `updateCustomer`, `createCustomer`, `updateUser`, `createUser` +haben `req.body` direkt oder via Spread an Prisma-`update/create` gereicht. +**Risiko:** +- Ein Angreifer mit `customers:update`-Permission konnte `portalPasswordHash` + (bcrypt-Hash!), `portalPasswordResetToken`, `consentHash`, `customerNumber` + direkt setzen +- Bei User-Update: `roleIds: [1]` übergeben → **Privilege Escalation** zum Admin +- `isActive: false` → andere User deaktivieren +**Fix:** Neue Whitelist-Helper `pickCustomerUpdate/Create`, `pickUserUpdate/Create` +in `utils/sanitize.ts`. Nur explizit erlaubte Felder gehen an Prisma durch. +Kritische Felder wie `portalPasswordHash`, `customerNumber`, `id`, `createdAt`, +`consentHash` sind **nicht** übernehmbar. + +#### 11. IDOR bei weiteren sensiblen Endpoints +Nach der ersten Runde kam der Agent auf **13 weitere IDOR-Stellen**: +- `GET /meters/:meterId/readings` → fremde Zählerstände +- `GET /emails/:emailId/attachments/*` → fremde Email-Anhänge +- `GET /customers/:customerId/emails` → fremde Email-Inhalte (CachedEmail) +- `GET /contracts/:contractId/emails` → fremde Email-Inhalte per Vertrag +- `GET /emails/:id` → einzelne Email lesen +- `GET /stressfrei-emails/:id` → leakt `emailPasswordEncrypted` +- weitere… + +**Fix:** `utils/accessControl.ts` deutlich ausgebaut um: +- `canAccessAddress` +- `canAccessBankCard` +- `canAccessIdentityDocument` +- `canAccessMeter` +- `canAccessStressfreiEmail` +- `canAccessCachedEmail` + +Diese Helper laden die Ressource, prüfen die customerId und delegieren an +`canAccessCustomer` (Portal-Isolation + Vollmachten). In allen kritischen +Endpoints vor dem eigentlichen Datenzugriff aufgerufen. + +Zusätzlich: `getEmail` (StressfreiEmail) filtert `emailPasswordEncrypted` +für Portal-Kunden explizit raus, selbst wenn sie zufällig Zugriff haben. + +### 🟡 Wichtig + +#### 12. Path-Traversal bei Backup-Namen +**Vorher:** `req.params.name` wurde direkt an `fs.readFile(path.join(backupDir, name))` +weitergereicht. `../` würde aus dem Backup-Verzeichnis ausbrechen. +**Fix:** Neuer `isValidBackupName()`-Guard: nur `[A-Za-z0-9_-]+`, kein `..`. + +#### 13. Path-Traversal bei GDPR-Proof-Download +**Vorher:** `path.join(uploads, request.proofDocument)` ohne Validation. +Wenn ein Angreifer den `proofDocument`-Pfad in der DB manipulieren könnte +(z.B. über Mass-Assignment – das haben wir aber oben gefixt), wäre arbitrary +file download möglich. +**Fix:** `path.resolve` auf den Pfad anwenden, prüfen dass das Ergebnis im +uploads-Verzeichnis liegt. + +--- + ## Nicht kritische Findings (Empfehlungen für später) ### 🟢 Token in Query-Parameter