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`