From e775b0e439c56b99002f4e5a14d87435160b4034 Mon Sep 17 00:00:00 2001 From: duffyduck Date: Tue, 21 Jul 2026 09:13:18 +0200 Subject: [PATCH] Pentest R121: Audit-Verify-Fehlalarm bei leerem resourceId beheben MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit POST /api/audit-logs/verify meldete ~73% der Einträge als "manipuliert" – kein echtes Tampering, sondern ein Serialisierungs- Bug in generateHash: resourceId war beim Schreiben oft undefined (Middleware-Einträge ohne Route-ID). JSON.stringify lässt einen undefined-Wert weg → Hash ohne resourceId-Key. In der DB steht der Wert aber als NULL; verifyIntegrity/rehashAll lasen null zurück und JSON.stringify({resourceId:null}) schrieb ihn rein → anderer Hash → Fehlalarm für jede Zeile mit leerem resourceId. Die vom Pentester gefundene durationMs-Korrelation war nur ein Proxy (Middleware- Einträge haben durationMs UND oft kein resourceId). Fix: generateHash lässt nullish resourceId weg – reproduziert exakt das historische Schreibverhalten (undefined → Key weg). Kein Caller übergibt je null explizit (verifiziert), daher matchen alle Bestands-Hashes ohne Rehash; Feldreihenfolge unverändert. Empirisch gegen echte DB bewiesen: aktuelle Hash-Ära (id>4141) 482/482 valide (vorher wurden alle 289 leeren-resourceId-Zeilen falsch geflaggt). Separat/vorbestehend (NICHT dieser Fix): Einträge vor Commit fd55742 ("complete new audit system") nutzen ein altes Hash-Schema und brauchen einmalig POST /api/audit-logs/rehash zum Re-Baselinen. Co-Authored-By: Claude Opus 4.7 --- backend/src/services/audit.service.ts | 30 ++++++++++++++++++++------- docs/todo.md | 21 +++++++++++++++++++ 2 files changed, 44 insertions(+), 7 deletions(-) diff --git a/backend/src/services/audit.service.ts b/backend/src/services/audit.service.ts index bd074f5e..716d23d2 100644 --- a/backend/src/services/audit.service.ts +++ b/backend/src/services/audit.service.ts @@ -91,16 +91,32 @@ function generateHash(data: { createdAt: Date; previousHash?: string | null; }): string { - const content = JSON.stringify({ + // WICHTIG – Pentest R121 (Verify-Fehlalarm, ~73% "manipuliert"): + // Beim Schreiben war `resourceId` bei middleware-generierten Einträgen + // oft `undefined`. JSON.stringify LÄSST einen undefined-Wert weg – der + // Hash wurde also OHNE `resourceId`-Key gebildet. In der DB landet der + // Wert aber als `NULL`. verifyIntegrity/rehashAll lasen ihn als `null` + // zurück, und `JSON.stringify({resourceId: null})` schreibt + // `"resourceId":null` REIN → anderer Hash → falscher Tamper-Alarm für + // jede Zeile mit leerem resourceId. + // + // Fix: nullish resourceId weglassen – reproduziert exakt das historische + // Schreibverhalten (undefined → Key weg). Kein Caller hat je `null` + // explizit übergeben (verifiziert), daher matchen ALLE Bestands-Hashes + // ohne Rehash. Feldreihenfolge bleibt identisch zur Alt-Serialisierung. + const payload: Record = { userEmail: data.userEmail, action: data.action, resourceType: data.resourceType, - resourceId: data.resourceId, - endpoint: data.endpoint, - createdAt: data.createdAt.toISOString(), - previousHash: data.previousHash || '', - }); - return crypto.createHash('sha256').update(content).digest('hex'); + }; + if (data.resourceId !== null && data.resourceId !== undefined) { + payload.resourceId = data.resourceId; + } + payload.endpoint = data.endpoint; + payload.createdAt = data.createdAt.toISOString(); + payload.previousHash = data.previousHash || ''; + + return crypto.createHash('sha256').update(JSON.stringify(payload)).digest('hex'); } /** diff --git a/docs/todo.md b/docs/todo.md index 0b0aa0d5..3284dc52 100644 --- a/docs/todo.md +++ b/docs/todo.md @@ -97,6 +97,27 @@ isolierte Instanz (keine Multi-Tenancy im Code), Provisioning + Abrechnung ## ✅ Erledigt +- [x] **🔴 Pentest R121 – Audit-Verify: Fehlalarm „manipuliert" bei leerem resourceId** + - `POST /api/audit-logs/verify` meldete ~73% der Einträge als + manipuliert. Kein echtes Tampering, sondern ein Bug in + `generateHash`: `resourceId` war beim Schreiben oft `undefined` + (middleware-Einträge ohne Route-ID). `JSON.stringify` LÄSST einen + undefined-Wert weg → Hash ohne `resourceId`-Key. In der DB landet + der Wert aber als `NULL`; `verifyIntegrity`/`rehashAll` lasen ihn + als `null` zurück und `JSON.stringify({resourceId:null})` schrieb + ihn REIN → anderer Hash → Fehlalarm für JEDE leere-resourceId-Zeile. + Die vom Pentester gefundene `durationMs`-Korrelation war ein Proxy: + middleware-Einträge (durationMs gesetzt) haben oft kein resourceId. + - Fix: `generateHash` lässt nullish `resourceId` weg – reproduziert + exakt das historische Schreibverhalten. Kein Caller übergibt je + `null` explizit (verifiziert) → alle Bestands-Hashes matchen ohne + Rehash. Empirisch gegen echte DB bewiesen: aktuelle Hash-Ära + (id>4141) 482/482 valide (vorher alle 289 null-Zeilen geflaggt). + - Separat/vorbestehend: Einträge VOR Commit `fd55742` („complete new + audit system") nutzen ein altes Hash-Schema und wurden nie neu + baselined → scheitern unabhängig davon. Remediation: einmalig + `POST /api/audit-logs/rehash` (Admin). Nicht Teil dieses Fixes. + - [x] **🔧 Pentest R120 – Audit-Log `/:id` mit nicht-numerischer ID → 500 statt 400** - Pentester stiess beim Suchen eines `verify-integrity`-Endpoints auf einen 500er. Ursache: `GET /api/audit-logs/verify` matcht `GET /:id`