From 23c530be46a110d6950020f938c3c504e880ac57 Mon Sep 17 00:00:00 2001 From: duffyduck Date: Fri, 17 Jul 2026 22:31:13 +0200 Subject: [PATCH] =?UTF-8?q?Pentest=20R120=20CRITICAL:=20IDOR=20auf=20Vertr?= =?UTF-8?q?agsbaum-Endpunkt=20schlie=C3=9Fen?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GET /api/contracts?tree=true&customerId= gab Portal-Usern den vollständigen Vertragsbaum beliebiger Fremdkunden zurück (Name, Kundennummer, Vertragsnummern, Tarife – HTTP 200). Der tree=true-Zweig returnte früh, bevor die Portal-User-customerIds- Filterung griff, die für die flache Liste im selben Handler läuft. Vorbestehender Bug; der includeDeactivated-Toggle machte ihn nur sichtbarer (auch archivierte Fremdverträge kamen mit). Live vom Pentester bestätigt, Gegentest ohne den Param = identisches Leck. Fix: canAccessCustomer(req, res, customerId) vor dem frühen Return. Prüft eigene ID + vertretene MIT Live-Vollmacht, sendet selbst die 403. Staff passiert unverändert. Gleiches Muster wie Pentest 56.3 bei update/delete. Flacher Listenpfad war nie betroffen: getAllContracts bevorzugt das serverseitig gesetzte customerIds-Scope gegenüber dem rohen customerId-Query-Param. Docs: SECURITY-HARDENING.md § Runde 120 + docs/todo.md. Co-Authored-By: Claude Opus 4.7 --- .../src/controllers/contract.controller.ts | 14 ++++++- docs/SECURITY-HARDENING.md | 40 +++++++++++++++++++ docs/todo.md | 21 ++++++++++ 3 files changed, 73 insertions(+), 2 deletions(-) diff --git a/backend/src/controllers/contract.controller.ts b/backend/src/controllers/contract.controller.ts index 93a9a742..94c0dd5f 100644 --- a/backend/src/controllers/contract.controller.ts +++ b/backend/src/controllers/contract.controller.ts @@ -10,7 +10,7 @@ import { ApiResponse, AuthRequest } from '../types/index.js'; import { logChange } from '../services/audit.service.js'; import { sanitizeContract, sanitizeContractStrict, sanitizeContracts, sanitizeContractsStrict, stripHtml, sanitizeNotes, validateContractDocumentType, validateOptionalIsoDate, isContractIdentifierField, validateContractIdentifier, validatePortalUsername } from '../utils/sanitize.js'; import { ApiError } from '../utils/apiError.js'; -import { canAccessContract } from '../utils/accessControl.js'; +import { canAccessContract, canAccessCustomer } from '../utils/accessControl.js'; import { maybeActivateOnDeliveryConfirmation, withContractDocumentLock } from '../services/contractStatusScheduler.service.js'; /** @@ -78,8 +78,18 @@ export async function getContracts(req: AuthRequest, res: Response): Promise). Ownership-Check nachgezogen – + // canAccessCustomer prüft eigene ID + vertretene MIT Live-Vollmacht + // und sendet selbst die 403-Response. Nicht-Portal-User (Staff) + // passieren unverändert. + if (!(await canAccessCustomer(req, res, customerIdNum))) return; const treeData = await contractService.getContractTreeForCustomer( - parseInt(customerId as string), + customerIdNum, includeDeactivated === 'true', ); res.json({ success: true, data: treeData } as ApiResponse); diff --git a/docs/SECURITY-HARDENING.md b/docs/SECURITY-HARDENING.md index f7bbfce2..b80a637e 100644 --- a/docs/SECURITY-HARDENING.md +++ b/docs/SECURITY-HARDENING.md @@ -654,6 +654,46 @@ Modus hängt sowieso an einer schon validierten Email-Stammdate --- +## 🔒 Runde 120 – KRITISCH: IDOR auf Vertragsbaum-Endpunkt (Live-Pentest-Fund) + +**Finding (CRITICAL):** Live nachgewiesen – ein Portal-User (Max, +`customerId=1`) konnte per +`GET /api/contracts?tree=true&customerId=2&includeDeactivated=true` +den vollständigen Vertragsbaum eines beliebigen Fremdkunden (Erika, +`customerId=2`) auslesen: fremder Name, Kundennummer, Vertragsnummern, +Provider und Tarife – HTTP 200. + +**Ursache:** Der `tree=true`-Zweig in `getContracts` returnte **früh**, +bevor die Portal-User-`customerIds`-Filterung griff, die für die flache +Vertragsliste weiter unten im selben Handler läuft. Der Zweig prüfte +schlicht nie, ob die angefragte `customerId` zum eingeloggten Portal-User +gehört. Vorbestehender Bug (nicht durch das R120-Feature „Deaktivierte +anzeigen" eingeführt) – der neue `includeDeactivated`-Toggle hat ihn nur +sichtbarer gemacht, weil dann sogar archivierte Fremdverträge +mitkamen. Gegentest ohne den Param: identisches Leck. + +**Fix:** `canAccessCustomer(req, res, customerId)` vor dem frühen Return +in [`contract.controller.ts`](../backend/src/controllers/contract.controller.ts). +Prüft eigene Customer-ID + vertretene Kunden MIT Live-Vollmacht +(`hasAuthorization`) und sendet selbst die 403. Nicht-Portal-User (Staff) +passieren unverändert. Gleiches Defense-in-Depth-Muster wie Pentest 56.3 +bei `updateContract`/`deleteContract`. + +**Zusatzbefund (Pentester):** In einem Staging-Vertrag lag ein +`