Pentest R121: Audit-Verify-Fehlalarm bei leerem resourceId beheben
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 <noreply@anthropic.com>
This commit is contained in:
@@ -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<string, unknown> = {
|
||||
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');
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -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`
|
||||
|
||||
Reference in New Issue
Block a user