From 28c91759df87515abb0abce49e528d611d40f02d Mon Sep 17 00:00:00 2001 From: duffyduck Date: Sun, 17 May 2026 08:51:52 +0200 Subject: [PATCH] Security-Hardening Runde 12: Information-Disclosure + Input-Validation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pentest Runde 7 (Anschlussrunde): MEDIUM – Interne Felder in Portal-Responses: - sanitizeCustomerStrict strippt zusätzlich portalTokenInvalidatedAt, portalLastLogin, portalPasswordMustChange, lastBirthdayGreetingYear, privacyPolicyPath, businessRegistrationPath, commercialRegisterPath. - Neue sanitizeContract/Strict + sanitizeContracts/Strict: entfernt portalPasswordEncrypted immer (nur über /password-Endpoint mit Audit abrufbar), für Portal-User zusätzlich commission/notes/nextReviewDate. - getContract + getContracts wählen je nach isCustomerPortal die passende Variante. Mitarbeiter sehen commission/notes weiterhin. LOW – Integer-Truncation bei IDs: parseInt('6abc') → 6 lief vorher durch. Neue Heuristik-Middleware unter /api: jedes Pfad-Segment, das mit Ziffer beginnt aber nicht aus reinen Ziffern besteht, wird mit 400 abgelehnt. Trifft alle Sub-Router ohne dass jede Route einzeln angefasst werden muss. INFO – Rate-Limit: Code-Stand limit=10 für Login, limit=5 für Password-Reset (lokal verifiziert: 11. failed login = 429). Pentester sah vermutlich noch älteren Build. Kein Code-Change. Live-verifiziert: - /customers/6abc → 400 "Ungültige ID im URL-Pfad" - /customers/3 → 200, /contracts/1abc/history → 400, normale Pfade OK - Portal-User /customers/3: keine portalLastLogin/portalPasswordMustChange/ portalTokenInvalidatedAt/etc. mehr in Response - Portal-User /contracts/15: keine commission/notes/portalPasswordEncrypted/ nextReviewDate - Admin /contracts/15: commission/notes/nextReviewDate sichtbar, portalPasswordEncrypted weg Co-Authored-By: Claude Opus 4.7 (1M context) --- .../src/controllers/contract.controller.ts | 15 +++- backend/src/index.ts | 19 +++++ backend/src/utils/sanitize.ts | 82 ++++++++++++++++++- docs/todo.md | 35 ++++++++ 4 files changed, 147 insertions(+), 4 deletions(-) diff --git a/backend/src/controllers/contract.controller.ts b/backend/src/controllers/contract.controller.ts index 78e2d9c1..3d45ea70 100644 --- a/backend/src/controllers/contract.controller.ts +++ b/backend/src/controllers/contract.controller.ts @@ -6,6 +6,7 @@ import * as contractHistoryService from '../services/contractHistory.service.js' import * as authorizationService from '../services/authorization.service.js'; import { ApiResponse, AuthRequest } from '../types/index.js'; import { logChange } from '../services/audit.service.js'; +import { sanitizeContract, sanitizeContractStrict, sanitizeContracts, sanitizeContractsStrict } from '../utils/sanitize.js'; import { canAccessContract } from '../utils/accessControl.js'; import { maybeActivateOnDeliveryConfirmation } from '../services/contractStatusScheduler.service.js'; @@ -46,9 +47,15 @@ export async function getContracts(req: AuthRequest, res: Response): Promise { 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- +// Validierung. Pentest Runde 7 (2026-05-17), LOW. +// +// `app.param()` greift nicht auf in Sub-Router gemounteten Routes, deshalb +// machen wir es als Pfad-Heuristik: jedes Segment, das mit einer Ziffer +// beginnt aber nicht aus reinen Ziffern besteht (`6abc`, `12foo`), wird als +// Tippfehler/Manipulation behandelt. +app.use('/api', (req, res, next) => { + for (const seg of req.path.split('/')) { + if (seg.length > 0 && /^\d/.test(seg) && !/^\d+$/.test(seg)) { + res.status(400).json({ success: false, error: 'Ungültige ID im URL-Pfad' }); + return; + } + } + next(); +}); + // Öffentliche Routes (OHNE Authentifizierung) app.use('/api/public/consent', consentPublicRoutes); diff --git a/backend/src/utils/sanitize.ts b/backend/src/utils/sanitize.ts index 55cba883..9c5941d9 100644 --- a/backend/src/utils/sanitize.ts +++ b/backend/src/utils/sanitize.ts @@ -17,6 +17,37 @@ const SENSITIVE_CUSTOMER_FIELDS = [ 'consentHash', ] as const; +// Zusätzliche Felder die Portal-User nicht in ihrer Customer-Response sehen +// sollen – Interne Session-/Workflow-State, kein direkter Auth-Bypass, aber +// unnötige Informationsleckage über den DB-Aufbau. +// Pentest Runde 7 (2026-05-17), MEDIUM. +const PORTAL_HIDDEN_CUSTOMER_FIELDS = [ + 'portalTokenInvalidatedAt', + 'portalLastLogin', + 'portalPasswordMustChange', + 'lastBirthdayGreetingYear', + // privacyPolicyPath etc. sind interne Datei-Pfade – Portal nutzt + // dedizierte PDF-Endpoints, nicht den Pfad direkt + 'privacyPolicyPath', + 'businessRegistrationPath', + 'commercialRegisterPath', +] as const; + +// Felder die im Contract NIE rausgehen dürfen (auch nicht an Mitarbeiter). +// portalPasswordEncrypted ist nur über den dedizierten /password-Endpoint +// (mit Audit-Log) abrufbar – im /contracts/:id selbst nutzlos. +const SENSITIVE_CONTRACT_FIELDS = [ + 'portalPasswordEncrypted', +] as const; + +// Zusätzliche Felder die Portal-User nicht sehen sollen (interne CRM-Daten). +// Pentest Runde 7 (2026-05-17): commission + notes leakten an Portal-User. +const PORTAL_HIDDEN_CONTRACT_FIELDS = [ + 'commission', + 'notes', + 'nextReviewDate', // Snooze-Workflow ist internes Cockpit-Feature +] as const; + const SENSITIVE_USER_FIELDS = [ 'password', 'passwordResetToken', @@ -43,14 +74,18 @@ export function sanitizeCustomer>(customer: T } /** - * Entfernt portalPasswordEncrypted zusätzlich zu den anderen sensiblen Feldern. - * Für Kontexte in denen der Caller KEIN Admin ist (z.B. Portal-Kunde). + * Entfernt portalPasswordEncrypted + portal-interne Workflow-Felder zusätzlich + * zu den allgemein sensiblen Feldern. Für Kontexte in denen der Caller KEIN + * Admin ist (z.B. Portal-Kunde). */ export function sanitizeCustomerStrict>(customer: T | null): T | null { if (!customer) return customer; const copy = sanitizeCustomer(customer) as Record | null; if (!copy) return null; delete copy.portalPasswordEncrypted; + for (const field of PORTAL_HIDDEN_CUSTOMER_FIELDS) { + delete copy[field]; + } return copy as T; } @@ -61,6 +96,49 @@ export function sanitizeCustomers>(customers: return customers.map((c) => sanitizeCustomer(c)).filter((c): c is T => c !== null); } +/** + * Sanitize Contract-Objekt für alle Caller. Entfernt das verschlüsselte + * Provider-Passwort (nur über den dedizierten /password-Endpoint mit + * Audit-Log abrufbar) und sanitisiert das embedded customer. + */ +export function sanitizeContract>(contract: T | null): T | null { + if (!contract) return contract; + const copy: Record = { ...contract }; + for (const field of SENSITIVE_CONTRACT_FIELDS) { + delete copy[field]; + } + if (copy.customer && typeof copy.customer === 'object') { + copy.customer = sanitizeCustomer(copy.customer as Record); + } + return copy as T; +} + +/** + * Sanitize Contract für Portal-User: zusätzlich werden interne CRM-Felder + * (Provision, Notizen, Snooze-Date) gestrippt und das embedded customer + * mit `sanitizeCustomerStrict` gefiltert. Pentest Runde 7 (2026-05-17). + */ +export function sanitizeContractStrict>(contract: T | null): T | null { + if (!contract) return contract; + const copy = sanitizeContract(contract) as Record | null; + if (!copy) return null; + for (const field of PORTAL_HIDDEN_CONTRACT_FIELDS) { + delete copy[field]; + } + if (copy.customer && typeof copy.customer === 'object') { + copy.customer = sanitizeCustomerStrict(copy.customer as Record); + } + return copy as T; +} + +export function sanitizeContracts>(contracts: T[]): T[] { + return contracts.map((c) => sanitizeContract(c)).filter((c): c is T => c !== null); +} + +export function sanitizeContractsStrict>(contracts: T[]): T[] { + return contracts.map((c) => sanitizeContractStrict(c)).filter((c): c is T => c !== null); +} + /** * Sanitize User-Objekt für API-Responses. */ diff --git a/docs/todo.md b/docs/todo.md index 37b388ad..3bf3140f 100644 --- a/docs/todo.md +++ b/docs/todo.md @@ -97,6 +97,41 @@ isolierte Instanz (keine Multi-Tenancy im Code), Provisioning + Abrechnung ## ✅ Erledigt +- [x] **🚨 Pentest Runde 7 (Anschlussrunde) – Information-Disclosure + Input-Validation** + - **MEDIUM – Interne Felder in Portal-Responses**: + * `sanitizeCustomerStrict` strippt jetzt zusätzlich + `portalTokenInvalidatedAt`, `portalLastLogin`, + `portalPasswordMustChange`, `lastBirthdayGreetingYear`, + `privacyPolicyPath`, `businessRegistrationPath`, + `commercialRegisterPath`. + * Neue `sanitizeContract` / `sanitizeContractStrict` / + `sanitizeContracts(Strict)`: entfernt + `portalPasswordEncrypted` (immer; ist nur über den dedizierten + `/password`-Endpoint mit Audit-Log abrufbar) und für Portal- + User zusätzlich `commission`, `notes`, `nextReviewDate`. + * `getContract` + `getContracts` rufen jetzt die passende + Sanitize-Variante je nach `req.user.isCustomerPortal` auf; + Mitarbeiter sehen weiterhin commission/notes (Admin-Workflow), + nur `portalPasswordEncrypted` ist generell entfernt (Klartext + nur über dedicated Endpoint). + * Live-verifiziert: Portal sieht 0 Leaks, Admin sieht + commission/notes weiterhin. + - **LOW – Integer-Truncation bei IDs**: + `parseInt('6abc')` → `6` hat alle Endpoints durchgewunken. + Neuer middleware in `index.ts`: jedes URL-Pfad-Segment unter + `/api`, das mit Ziffer beginnt aber nicht aus reinen Ziffern + besteht, wird mit HTTP 400 abgelehnt. Heuristik trifft alle + `/resource/(\D+)`-Patterns ohne dass jeder einzelne + Sub-Router angefasst werden muss. + * Live-verifiziert: `/customers/6abc` → 400 mit klarer Meldung, + `/customers/3` weiterhin 200, `/contracts/1abc/history` + → 400, normaler Pfade `/audit-logs/customer/3` → 200. + - **INFO – Login-Rate-Limit „nach 6 nicht aktiv"**: + Code-Stand `limit: 10` für `loginRateLimiter`, lokal verifiziert: + 11. Versuch = 429. Pentester sah vermutlich noch alten Build + oder eine andere Lokation (PW-Reset hat `limit: 5`). Kein + Code-Change. + - [x] **🛠 Rate-Limit-Sperren: Admin-UI zum Freigeben** - Bei einer Pentest-Runde hat der Tester sich selbst durch zu viele Login-Versuche ausgesperrt → ohne Container-Restart kein Weg zurück.