Pentest R120 CRITICAL: IDOR auf Vertragsbaum-Endpunkt schließen
GET /api/contracts?tree=true&customerId=<fremd> 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 <noreply@anthropic.com>
This commit is contained in:
@@ -10,7 +10,7 @@ import { ApiResponse, AuthRequest } from '../types/index.js';
|
|||||||
import { logChange } from '../services/audit.service.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 { sanitizeContract, sanitizeContractStrict, sanitizeContracts, sanitizeContractsStrict, stripHtml, sanitizeNotes, validateContractDocumentType, validateOptionalIsoDate, isContractIdentifierField, validateContractIdentifier, validatePortalUsername } from '../utils/sanitize.js';
|
||||||
import { ApiError } from '../utils/apiError.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';
|
import { maybeActivateOnDeliveryConfirmation, withContractDocumentLock } from '../services/contractStatusScheduler.service.js';
|
||||||
|
|
||||||
/**
|
/**
|
||||||
@@ -78,8 +78,18 @@ export async function getContracts(req: AuthRequest, res: Response): Promise<voi
|
|||||||
|
|
||||||
// Baumstruktur für Kundenansicht
|
// Baumstruktur für Kundenansicht
|
||||||
if (tree === 'true' && customerId) {
|
if (tree === 'true' && customerId) {
|
||||||
|
const customerIdNum = parseInt(customerId as string);
|
||||||
|
// Pentest R120 (CRITICAL IDOR): Der tree=true-Zweig returnte früh,
|
||||||
|
// BEVOR die Portal-User-customerIds-Filterung unten für die flache
|
||||||
|
// Liste griff. Ein Portal-User konnte damit den vollständigen
|
||||||
|
// Vertragsbaum eines beliebigen Fremdkunden lesen
|
||||||
|
// (?tree=true&customerId=<fremd>). 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(
|
const treeData = await contractService.getContractTreeForCustomer(
|
||||||
parseInt(customerId as string),
|
customerIdNum,
|
||||||
includeDeactivated === 'true',
|
includeDeactivated === 'true',
|
||||||
);
|
);
|
||||||
res.json({ success: true, data: treeData } as ApiResponse);
|
res.json({ success: true, data: treeData } as ApiResponse);
|
||||||
|
|||||||
@@ -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
|
||||||
|
`<script>`-Test-Artefakt in `providerName`/`tariffName`. Nicht
|
||||||
|
exploitierbar – keine der 9 `dangerouslySetInnerHTML`-Stellen im Frontend
|
||||||
|
rendert Vertrags-Provider-/Tarif-Namen, React escaped sie als Text.
|
||||||
|
Schreibpfad entschärft neue Werte zusätzlich per `sanitizeContractBody`
|
||||||
|
(`stripHtml`); der Altwert stammt aus DB-Direkteingabe.
|
||||||
|
|
||||||
|
**Lessons Learned (Pentester-seitig):** Der Fund verzögerte sich, weil
|
||||||
|
zunächst gegen den falschen Host getestet wurde
|
||||||
|
(`kundencenter.hacker-net.de` = PROD statt
|
||||||
|
`kundencenter-stage.stressfrei-wechseln.de`). Auf dem korrekten
|
||||||
|
Staging-Host reproduzierte sich das Finding sofort.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
## 🔒 Runde 110 – Mass-Assignment-Whitelist auf 7 Katalog-Endpunkten
|
## 🔒 Runde 110 – Mass-Assignment-Whitelist auf 7 Katalog-Endpunkten
|
||||||
|
|
||||||
**Finding (MEDIUM):** Der Pentester hat live nachgewiesen, dass
|
**Finding (MEDIUM):** Der Pentester hat live nachgewiesen, dass
|
||||||
|
|||||||
@@ -97,6 +97,27 @@ isolierte Instanz (keine Multi-Tenancy im Code), Provisioning + Abrechnung
|
|||||||
|
|
||||||
## ✅ Erledigt
|
## ✅ Erledigt
|
||||||
|
|
||||||
|
- [x] **🔴 Pentest R120 – CRITICAL IDOR: Vertragsbaum fremder Kunden lesbar**
|
||||||
|
- `GET /api/contracts?tree=true&customerId=<fremd>` returnte für
|
||||||
|
Portal-User den vollständigen Vertragsbaum eines beliebigen
|
||||||
|
Fremdkunden (Name, Kundennummer, Vertragsnummern, Tarife). Der
|
||||||
|
`tree=true`-Zweig im Controller returnte **früh**, bevor die
|
||||||
|
Portal-User-`customerIds`-Filterung unten für die flache Liste
|
||||||
|
griff. Vorbestehender Bug – der Toggle „Deaktivierte anzeigen"
|
||||||
|
(siehe unten) hat ihn nur sichtbarer gemacht, weil selbst
|
||||||
|
archivierte Fremdverträge mit auftauchten.
|
||||||
|
- Fix: `canAccessCustomer(req, res, customerId)` vor dem frühen
|
||||||
|
Return ([contract.controller.ts:80](../backend/src/controllers/contract.controller.ts#L80)).
|
||||||
|
Prüft eigene Customer-ID + vertretene MIT Live-Vollmacht, sendet
|
||||||
|
selbst die 403. Staff (Nicht-Portal) passiert unverändert. Gleiches
|
||||||
|
Muster wie Pentest 56.3 bei update/delete.
|
||||||
|
- Zusatzbefund (Pentester): liegen gebliebenes `<script>`-Test-
|
||||||
|
Artefakt in `providerName`/`tariffName` eines Staging-Vertrags.
|
||||||
|
Nicht exploitierbar – keine der 9 `dangerouslySetInnerHTML`-Stellen
|
||||||
|
rendert Vertrags-Provider/Tarif-Namen, React escaped sie als
|
||||||
|
Text. Neue Writes werden zusätzlich per `sanitizeContractBody`
|
||||||
|
(stripHtml) entschärft; der Altwert stammt aus DB-Direkteingabe.
|
||||||
|
|
||||||
- [x] **👁 Kundenansicht: Toggle „Deaktivierte Verträge anzeigen"**
|
- [x] **👁 Kundenansicht: Toggle „Deaktivierte Verträge anzeigen"**
|
||||||
- Der Vertragsbaum beim Kunden (`CustomerDetail` → Tab Verträge)
|
- Der Vertragsbaum beim Kunden (`CustomerDetail` → Tab Verträge)
|
||||||
blendete `DEACTIVATED`-Verträge komplett aus. Da der jeweils
|
blendete `DEACTIVATED`-Verträge komplett aus. Da der jeweils
|
||||||
|
|||||||
Reference in New Issue
Block a user