From 4ca91eb7109ea542726504e1ea49e281505ec1a4 Mon Sep 17 00:00:00 2001 From: duffyduck Date: Fri, 24 Apr 2026 09:59:37 +0200 Subject: [PATCH] Security-Hardening Runde 4: 9 Live-IDORs + Error-Handler MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Live-Pentest gegen Dev-Server mit Portal-Token deckte auf, dass customer.* und gdpr.* Endpoints nur den Data-Sanitizer, aber KEINEN canAccessCustomer-Check hatten. Ein Portal-Kunde mit customers:read konnte per ID-Manipulation komplette Fremddatensätze auslesen. - customer.controller.getCustomer + getAddresses + getBankCards + getDocuments + getMeters + getRepresentatives + getPortalSettings: canAccessCustomer - gdpr.controller.getCustomerConsents + getAuthorizations + checkConsentStatus: canAccessCustomer - createAddress/createBankCard/createDocument/createMeter (customerId aus URL): canAccessCustomer (Defense-in-Depth – wird aktuell schon per Permission geblockt, aber im Controller ungeschützt) - Global Error-Handler: err.status respektieren (PayloadTooLargeError → 413 "Anfrage zu groß", SyntaxError → 400 "Ungültiges JSON" statt pauschal 500) Live-verifiziert: ✓ /api/customers/4 als Portal → 200 VORHER, 403 NACHHER ✓ 9 andere IDOR-Endpoints gleiches Muster ✓ Eigene Daten (/api/customers/1) weiter 200 ✓ 12 MB Body → 413, malformed JSON → 400 Co-Authored-By: Claude Opus 4.7 (1M context) --- .../src/controllers/customer.controller.ts | 62 +++++++++++-------- backend/src/controllers/gdpr.controller.ts | 4 ++ backend/src/index.ts | 13 +++- backend/todo.md | 5 ++ 4 files changed, 56 insertions(+), 28 deletions(-) diff --git a/backend/src/controllers/customer.controller.ts b/backend/src/controllers/customer.controller.ts index 85878031..515a1229 100644 --- a/backend/src/controllers/customer.controller.ts +++ b/backend/src/controllers/customer.controller.ts @@ -16,6 +16,7 @@ import { canAccessAddress, canAccessBankCard, canAccessIdentityDocument, + canAccessCustomer, } from '../utils/accessControl.js'; // Customer CRUD @@ -44,7 +45,9 @@ export async function getCustomers(req: AuthRequest, res: Response): Promise { try { - const customer = await customerService.getCustomerById(parseInt(req.params.id)); + const customerId = parseInt(req.params.id); + if (!(await canAccessCustomer(req, res, customerId))) return; + const customer = await customerService.getCustomerById(customerId); if (!customer) { res.status(404).json({ success: false, error: 'Kunde nicht gefunden' } as ApiResponse); return; @@ -185,18 +188,21 @@ export async function deleteCustomer(req: Request, res: Response): Promise } // Addresses -export async function getAddresses(req: Request, res: Response): Promise { +export async function getAddresses(req: AuthRequest, res: Response): Promise { try { - const addresses = await customerService.getCustomerAddresses(parseInt(req.params.customerId)); + const customerId = parseInt(req.params.customerId); + if (!(await canAccessCustomer(req, res, customerId))) return; + const addresses = await customerService.getCustomerAddresses(customerId); res.json({ success: true, data: addresses } as ApiResponse); } catch (error) { res.status(500).json({ success: false, error: 'Fehler beim Laden der Adressen' } as ApiResponse); } } -export async function createAddress(req: Request, res: Response): Promise { +export async function createAddress(req: AuthRequest, res: Response): Promise { try { const customerId = parseInt(req.params.customerId); + if (!(await canAccessCustomer(req, res, customerId))) return; const address = await customerService.createAddress(customerId, req.body); await logChange({ req, action: 'CREATE', resourceType: 'Address', @@ -298,22 +304,22 @@ export async function deleteAddress(req: Request, res: Response): Promise } // Bank Cards -export async function getBankCards(req: Request, res: Response): Promise { +export async function getBankCards(req: AuthRequest, res: Response): Promise { try { + const customerId = parseInt(req.params.customerId); + if (!(await canAccessCustomer(req, res, customerId))) return; const showInactive = req.query.showInactive === 'true'; - const cards = await customerService.getCustomerBankCards( - parseInt(req.params.customerId), - showInactive - ); + const cards = await customerService.getCustomerBankCards(customerId, showInactive); res.json({ success: true, data: cards } as ApiResponse); } catch (error) { res.status(500).json({ success: false, error: 'Fehler beim Laden der Bankkarten' } as ApiResponse); } } -export async function createBankCard(req: Request, res: Response): Promise { +export async function createBankCard(req: AuthRequest, res: Response): Promise { try { const customerId = parseInt(req.params.customerId); + if (!(await canAccessCustomer(req, res, customerId))) return; const card = await customerService.createBankCard(customerId, req.body); await logChange({ req, action: 'CREATE', resourceType: 'BankCard', @@ -410,22 +416,22 @@ export async function deleteBankCard(req: Request, res: Response): Promise } // Identity Documents -export async function getDocuments(req: Request, res: Response): Promise { +export async function getDocuments(req: AuthRequest, res: Response): Promise { try { + const customerId = parseInt(req.params.customerId); + if (!(await canAccessCustomer(req, res, customerId))) return; const showInactive = req.query.showInactive === 'true'; - const docs = await customerService.getCustomerDocuments( - parseInt(req.params.customerId), - showInactive - ); + const docs = await customerService.getCustomerDocuments(customerId, showInactive); res.json({ success: true, data: docs } as ApiResponse); } catch (error) { res.status(500).json({ success: false, error: 'Fehler beim Laden der Ausweise' } as ApiResponse); } } -export async function createDocument(req: Request, res: Response): Promise { +export async function createDocument(req: AuthRequest, res: Response): Promise { try { const customerId = parseInt(req.params.customerId); + if (!(await canAccessCustomer(req, res, customerId))) return; const doc = await customerService.createDocument(customerId, req.body); await logChange({ req, action: 'CREATE', resourceType: 'IdentityDocument', @@ -528,22 +534,22 @@ export async function deleteDocument(req: Request, res: Response): Promise } // Meters -export async function getMeters(req: Request, res: Response): Promise { +export async function getMeters(req: AuthRequest, res: Response): Promise { try { + const customerId = parseInt(req.params.customerId); + if (!(await canAccessCustomer(req, res, customerId))) return; const showInactive = req.query.showInactive === 'true'; - const meters = await customerService.getCustomerMeters( - parseInt(req.params.customerId), - showInactive - ); + const meters = await customerService.getCustomerMeters(customerId, showInactive); res.json({ success: true, data: meters } as ApiResponse); } catch (error) { res.status(500).json({ success: false, error: 'Fehler beim Laden der Zähler' } as ApiResponse); } } -export async function createMeter(req: Request, res: Response): Promise { +export async function createMeter(req: AuthRequest, res: Response): Promise { try { const customerId = parseInt(req.params.customerId); + if (!(await canAccessCustomer(req, res, customerId))) return; const meter = await customerService.createMeter(customerId, req.body); await logChange({ req, action: 'CREATE', resourceType: 'Meter', @@ -847,9 +853,11 @@ export async function markReadingTransferred(req: AuthRequest, res: Response): P // ==================== PORTAL SETTINGS ==================== -export async function getPortalSettings(req: Request, res: Response): Promise { +export async function getPortalSettings(req: AuthRequest, res: Response): Promise { try { - const settings = await customerService.getPortalSettings(parseInt(req.params.customerId)); + const customerId = parseInt(req.params.customerId); + if (!(await canAccessCustomer(req, res, customerId))) return; + const settings = await customerService.getPortalSettings(customerId); if (!settings) { res.status(404).json({ success: false, error: 'Kunde nicht gefunden' } as ApiResponse); return; @@ -977,10 +985,12 @@ export async function getPortalPassword(req: Request, res: Response): Promise { +export async function getRepresentatives(req: AuthRequest, res: Response): Promise { try { + const customerId = parseInt(req.params.customerId); + if (!(await canAccessCustomer(req, res, customerId))) return; // Wer kann diesen Kunden vertreten (representedBy)? - const representedBy = await customerService.getRepresentedByList(parseInt(req.params.customerId)); + const representedBy = await customerService.getRepresentedByList(customerId); res.json({ success: true, data: representedBy } as ApiResponse); } catch (error) { res.status(500).json({ diff --git a/backend/src/controllers/gdpr.controller.ts b/backend/src/controllers/gdpr.controller.ts index 8102ca03..bf70d41e 100644 --- a/backend/src/controllers/gdpr.controller.ts +++ b/backend/src/controllers/gdpr.controller.ts @@ -4,6 +4,7 @@ import * as gdprService from '../services/gdpr.service.js'; import * as consentService from '../services/consent.service.js'; import * as consentPublicService from '../services/consent-public.service.js'; import * as appSettingService from '../services/appSetting.service.js'; +import { canAccessCustomer } from '../utils/accessControl.js'; import { createAuditLog, logChange } from '../services/audit.service.js'; import { ConsentType, DeletionRequestStatus } from '@prisma/client'; import prisma from '../lib/prisma.js'; @@ -229,6 +230,7 @@ export async function getDashboardStats(req: AuthRequest, res: Response) { export async function getCustomerConsents(req: AuthRequest, res: Response) { try { const customerId = parseInt(req.params.customerId); + if (!(await canAccessCustomer(req, res, customerId))) return; const consents = await consentService.getCustomerConsents(customerId); // Labels hinzufügen @@ -251,6 +253,7 @@ export async function getCustomerConsents(req: AuthRequest, res: Response) { export async function checkConsentStatus(req: AuthRequest, res: Response) { try { const customerId = parseInt(req.params.customerId); + if (!(await canAccessCustomer(req, res, customerId))) return; const result = await consentService.hasFullConsent(customerId); res.json({ success: true, data: result }); } catch (error) { @@ -799,6 +802,7 @@ export async function sendAuthorizationRequest(req: AuthRequest, res: Response) export async function getAuthorizations(req: AuthRequest, res: Response) { try { const customerId = parseInt(req.params.customerId); + if (!(await canAccessCustomer(req, res, customerId))) return; // Sicherstellen dass Einträge für alle aktiven Vertreter existieren await authorizationService.ensureAuthorizationEntries(customerId); const authorizations = await authorizationService.getAuthorizationsForCustomer(customerId); diff --git a/backend/src/index.ts b/backend/src/index.ts index fa03ef4b..f256f80f 100644 --- a/backend/src/index.ts +++ b/backend/src/index.ts @@ -154,9 +154,18 @@ if (process.env.NODE_ENV === 'production') { } // Error handling -app.use((err: Error, req: express.Request, res: express.Response, next: express.NextFunction) => { +// body-parser wirft 413 (PayloadTooLargeError) bzw. 400 (SyntaxError) mit einem +// `status`-Feld. Ohne Respektierung werden legitime Client-Fehler als 500 +// kaschiert und landen als "Interner Serverfehler" beim User. +app.use((err: Error & { status?: number; type?: string }, req: express.Request, res: express.Response, next: express.NextFunction) => { console.error(err.stack); - res.status(500).json({ success: false, error: 'Interner Serverfehler' }); + const status = typeof err.status === 'number' && err.status >= 400 && err.status < 600 ? err.status : 500; + let message = 'Interner Serverfehler'; + if (status === 413) message = 'Anfrage zu groß'; + else if (status === 400 && (err.type === 'entity.parse.failed' || err instanceof SyntaxError)) { + message = 'Ungültiges JSON'; + } + res.status(status).json({ success: false, error: message }); }); app.listen(PORT, () => { diff --git a/backend/todo.md b/backend/todo.md index 222e6adc..60a5f25b 100644 --- a/backend/todo.md +++ b/backend/todo.md @@ -122,6 +122,11 @@ isolierte Instanz (keine Multi-Tenancy im Code), Provisioning + Abrechnung - Provider/Tariff-GETs: `requirePermission('providers:read')` (Portal-Kunden sehen Provider-Liste nicht mehr) - SMTP-Header-Injection: zentrale CRLF-Validierung in `smtpService.sendEmail` (schützt alle Caller) - bcrypt cost 10 → 12 (OWASP 2026) + - **Runde 4 – Live-Tests gegen Dev-Server deckten 9 weitere IDORs auf:** + - `getCustomer` + `getAddresses`/`getBankCards`/`getDocuments`/`getMeters`/`getRepresentatives`/`getPortalSettings` hatten NUR Daten-Sanitizer aber KEINEN `canAccessCustomer`-Check + - `gdpr.getCustomerConsents` + `getAuthorizations` + `checkConsentStatus` ebenso ungeschützt + - Portal-Kunde konnte live per `GET /api/customers/` kompletten Fremdkunden-Datensatz auslesen → jetzt 403 + - Error-Handler: `err.status` wird jetzt respektiert (413/400 statt pauschalem 500) - Deployment-Checkliste komplett - [x] **🎉 Version 1.0.0 Feinschliff: Passwort-Reset + Rate-Limiting + Auto-Geburtstagsgrüße**