From 373fab8e836fd508288eabc2a6779a0d8c6f39b7 Mon Sep 17 00:00:00 2001 From: duffyduck Date: Mon, 18 May 2026 18:43:45 +0200 Subject: [PATCH] =?UTF-8?q?Security-Hardening=20Runde=2016:=20KRITISCH=20?= =?UTF-8?q?=E2=80=93=20Update-Responses=20sanitisieren?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pentest Runde 15: 20.3 KRITISCH: PUT /customers/:id gab portalPasswordHash (bcrypt $2a$12$…) im Response zurück. updateCustomer reichte das rohe Service-Output ohne sanitize-Aufruf durch. 20.4 HOCH (gleiche Klasse): PUT-Response leakte portalPasswordResetToken, portalPasswordMustChange, consentHash, portalTokenInvalidatedAt. Fix: - updateCustomer + createCustomer rufen sanitizeCustomer bzw. sanitizeCustomerStrict je nach customers:update-Permission. - updateContract + createContract + createFollowUp + createRenewal analog mit sanitizeContract / sanitizeContractStrict je nach isCustomerPortal. - portalPasswordMustChange + portalTokenInvalidatedAt von PORTAL_HIDDEN_CUSTOMER_FIELDS zu SENSITIVE_CUSTOMER_FIELDS hochgezogen → greift auch in normaler sanitizeCustomer (Admin-Sicht). Live-verifiziert: - Admin PUT /customers/3 → 0 Leaks von Hash/Token/Expires/MustChange/ consentHash/TokenInvalidatedAt; portalPasswordEncrypted bleibt für Admin sichtbar (UI-Workflow, separater Endpoint mit Audit) - POST /customers → 0 Leaks - Portal-User GET /customers/3 → 0 Leaks auch bei portalPasswordEncrypted/notes Co-Authored-By: Claude Opus 4.7 (1M context) --- .../src/controllers/contract.controller.ts | 22 ++++++++++---- .../src/controllers/customer.controller.ts | 18 ++++++++++-- backend/src/utils/sanitize.ts | 12 ++++++-- docs/todo.md | 29 +++++++++++++++++++ 4 files changed, 72 insertions(+), 9 deletions(-) diff --git a/backend/src/controllers/contract.controller.ts b/backend/src/controllers/contract.controller.ts index 4061a145..7c3682ef 100644 --- a/backend/src/controllers/contract.controller.ts +++ b/backend/src/controllers/contract.controller.ts @@ -109,7 +109,7 @@ export async function getContract(req: AuthRequest, res: Response): Promise { +export async function createContract(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). @@ -129,7 +129,9 @@ export async function createContract(req: Request, res: Response): Promise label: `Vertrag ${contract.contractNumber} angelegt`, customerId: contract.customerId, }); - res.status(201).json({ success: true, data: contract } as ApiResponse); + const isPortal = !!req.user?.isCustomerPortal; + const sanitized = isPortal ? sanitizeContractStrict(contract as any) : sanitizeContract(contract as any); + res.status(201).json({ success: true, data: sanitized } as ApiResponse); } catch (error) { res.status(400).json({ success: false, @@ -201,7 +203,13 @@ export async function updateContract(req: AuthRequest, res: Response): Promise label: `Kunde ${customer.customerNumber} angelegt (${customer.firstName} ${customer.lastName})`, customerId: customer.id, }); - res.status(201).json({ success: true, data: customer } as ApiResponse); + // Response sanitisieren (Pentest Runde 15, 20.3/20.4): die Service- + // Funktion gibt das rohe DB-Objekt mit portalPasswordHash + Reset-Token + // zurück. Ohne sanitize-Aufruf leakte das beim Erstellen + Update. + const canSeePasswords = (req as AuthRequest).user?.permissions?.includes('customers:update') ?? false; + const sanitized = canSeePasswords + ? sanitizeCustomer(customer as any) + : sanitizeCustomerStrict(customer as any); + res.status(201).json({ success: true, data: sanitized } as ApiResponse); } catch (error) { res.status(400).json({ success: false, @@ -169,7 +176,14 @@ export async function updateCustomer(req: Request, res: Response): Promise } } - res.json({ success: true, data: customer } as ApiResponse); + // Response sanitisieren – sonst leakt portalPasswordHash + + // portalPasswordResetToken + consentHash + portalPasswordMustChange. + // Pentest Runde 15 (20.3 KRITISCH, 20.4 HOCH). + const canSeePasswords = (req as AuthRequest).user?.permissions?.includes('customers:update') ?? false; + const sanitized = canSeePasswords + ? sanitizeCustomer(customer as any) + : sanitizeCustomerStrict(customer as any); + res.json({ success: true, data: sanitized } as ApiResponse); } catch (error) { console.error('Update customer error:', error); res.status(400).json({ diff --git a/backend/src/utils/sanitize.ts b/backend/src/utils/sanitize.ts index 734aa2a5..bff8d31c 100644 --- a/backend/src/utils/sanitize.ts +++ b/backend/src/utils/sanitize.ts @@ -15,6 +15,14 @@ const SENSITIVE_CUSTOMER_FIELDS = [ // braucht, holt ihn über GET /gdpr/customer/:id/consent-status (eigener // Endpoint mit canAccessCustomer-Check). Pentest Runde 5 (2026-05-16). 'consentHash', + // Session-/OTP-State – Pentest Runde 15 (2026-05-18, 20.4 HOCH): zeigt + // einem externen Beobachter, ob ein Kunde gerade im OTP-Flow ist und + // wann zuletzt seine Tokens invalidiert wurden. Reiner Info-Leak ohne + // Auth-Bypass, aber unnötig. Wenn Admin diese Information legitim + // braucht (z.B. UI-Hinweis "OTP wurde noch nicht eingelöst"), führen + // wir bei Bedarf einen eigenen Endpoint ein. + 'portalPasswordMustChange', + 'portalTokenInvalidatedAt', ] as const; // Zusätzliche Felder die Portal-User nicht in ihrer Customer-Response sehen @@ -22,9 +30,9 @@ const SENSITIVE_CUSTOMER_FIELDS = [ // unnötige Informationsleckage über den DB-Aufbau. // Pentest Runde 7 (2026-05-17), MEDIUM. const PORTAL_HIDDEN_CUSTOMER_FIELDS = [ - 'portalTokenInvalidatedAt', + // portalTokenInvalidatedAt + portalPasswordMustChange sind jetzt in + // SENSITIVE_CUSTOMER_FIELDS (immer raus), nicht mehr nur für Portal. 'portalLastLogin', - 'portalPasswordMustChange', 'lastBirthdayGreetingYear', // privacyPolicyPath etc. sind interne Datei-Pfade – Portal nutzt // dedizierte PDF-Endpoints, nicht den Pfad direkt diff --git a/docs/todo.md b/docs/todo.md index 86f25969..e235822f 100644 --- a/docs/todo.md +++ b/docs/todo.md @@ -97,6 +97,35 @@ isolierte Instanz (keine Multi-Tenancy im Code), Provisioning + Abrechnung ## ✅ Erledigt +- [x] **🚨 Pentest Runde 15 – KRITISCH: portalPasswordHash in PUT/POST-Response** + - **20.3 KRITISCH**: `PUT /customers/:id` gab den vollen bcrypt-Hash + (`$2a$12$…`) zurück, weil `updateCustomer` Service-Output ohne + sanitize-Aufruf direkt durchreichte. GET-Endpoints waren dicht, + die Update-Response nicht. **20.4 HOCH** gleicher Klasse: + `portalPasswordResetToken`, `consentHash`, + `portalPasswordMustChange`, `portalTokenInvalidatedAt` leakten + ebenfalls über PUT/POST. + - **Fix**: + * `updateCustomer` + `createCustomer` rufen jetzt + `sanitizeCustomer`/`sanitizeCustomerStrict` auf den Service- + Output (je nach `customers:update`-Permission). + * `updateContract` + `createContract` + `createFollowUp` + + `createRenewal` analog mit `sanitizeContract`/Strict + (Portal-Hint via `req.user.isCustomerPortal`). + * `portalPasswordMustChange` und `portalTokenInvalidatedAt` + zusätzlich von `PORTAL_HIDDEN_CUSTOMER_FIELDS` zu + `SENSITIVE_CUSTOMER_FIELDS` hochgezogen – damit greift der + Schutz auch bei der normalen `sanitizeCustomer`-Variante + (Admin-Sicht). Auch Pentester-Empfehlung in HOCH-Klasse. + - **Live-verifiziert**: + * Admin `PUT /customers/3 {firstName:…}` → 0 Leaks bei + portalPasswordHash/ResetToken/Expires/MustChange/consentHash/ + TokenInvalidatedAt; `portalPasswordEncrypted` bleibt für + Admin sichtbar (UI-Workflow) + * Portal-User `GET /customers/3` → 0 Leaks auch bei + portalPasswordEncrypted/notes + * `POST /customers` (create) ebenfalls dicht + - [x] **🛟 Admin-Rescue-Script (PW-Reset direkt in DB + Rate-Limit-Reset)** - Use Case: Admin sperrt sich aus (z.B. `admin@admin.com` ist keine echte E-Mail → Passwort-vergessen-Flow kann keine Mail