Security-Hardening Runde 2: Zip-Slip, Mass Assignment, weitere IDORs, Path-Traversal
Nach der ersten Runde habe ich parallel 3 Audit-Agents auf die Codebase angesetzt. Die fanden noch eine Menge: Zip-Slip, Mass Assignment inkl. Privilege Escalation, 13 weitere IDOR-Stellen, 2x Path-Traversal. Alles gefixt. Details + Angriffsvektoren in docs/SECURITY-REVIEW.md. 🔴 KRITISCH gefixt: 1. Zip-Slip im Backup-Upload: extractAllTo() entpackte bösartige ZIPs ohne Pfad-Validierung. Ein Angreifer mit Admin-Zugang hätte mit einem ZIP mit Entries wie ../../etc/crontab das ganze Filesystem überschreiben können. Jetzt wird jeder ZIP-Entry einzeln validiert (path.resolve, starts-with-Check). Absolute Pfade + Null-Bytes werden abgelehnt. 2. Mass Assignment bei Customer/User Controllers: - updateCustomer/createCustomer: req.body ging komplett an Prisma. Angreifer konnte portalPasswordHash, portalPasswordResetToken, consentHash, customerNumber direkt setzen. - updateUser/createUser: roleIds und isActive waren übernehmbar. **Privilege Escalation**: normaler Mitarbeiter konnte sich Admin-Rechte durch PUT /users/:id mit {"roleIds":[1]} geben, oder andere User deaktivieren. Fix: Neue Whitelist-Helper pickCustomerCreate/Update, pickUserCreate/Update in utils/sanitize.ts. Nur erlaubte Felder werden durchgelassen. 3. IDOR bei 13 weiteren Endpoints (neben denen aus Runde 1): - GET /meters/:meterId/readings - GET /emails/:emailId/attachments/:filename - GET /emails/:emailId/attachments (Liste) - GET /customers/:customerId/emails - GET /contracts/:contractId/emails - GET /emails/:id (einzelne Email) - GET /stressfrei-emails/:id (leakte emailPasswordEncrypted) - weitere… Fix: accessControl.ts ausgebaut um canAccessAddress, canAccessBankCard, canAccessIdentityDocument, canAccessMeter, canAccessStressfreiEmail, canAccessCachedEmail. In allen betroffenen Endpoints angewendet. 🟡 WICHTIG gefixt: 4. Path-Traversal bei Backup-Name (GET /settings/backup/:name/*): req.params.name wurde ohne Filter in path.join. Neuer isValidBackupName() erlaubt nur [A-Za-z0-9_-]+ ohne "..". 5. Path-Traversal bei GDPR-Proof-Download: proofDocument-Pfad aus DB wurde ohne Validation gejoined. Jetzt path.resolve + starts-with-uploads-Check. Neue/erweiterte Files: - backend/src/utils/accessControl.ts - 6 neue can-Access-Helper - backend/src/utils/sanitize.ts - 4 neue Whitelist-pick-Helper - docs/SECURITY-REVIEW.md - Runde 2 dokumentiert Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -1,5 +1,9 @@
|
||||
# Security-Review vor 1.0.0
|
||||
|
||||
> **Version 2** – dieser Review wurde in 2 Runden durchgeführt.
|
||||
> Runde 1: erste kritische Findings (CORS, Helmet, JWT-Fallback, grobes IDOR, XSS, Data Exposure).
|
||||
> Runde 2 (weiter unten): **Deep-Dive** mit parallelen Audit-Agents – fand weitere IDOR-Stellen, Mass Assignment, Zip-Slip, Path-Traversal.
|
||||
|
||||
Systematischer Review des Codebase mit Fokus auf Produktions-Hardening
|
||||
vor öffentlichem Deployment (hinter HTTPS-Proxy).
|
||||
|
||||
@@ -83,6 +87,75 @@ Auth-Middleware prüft bei Portal-Sessions diesen Timestamp gegen `token.iat`.
|
||||
**Fix:** `express.json({ limit: '5mb' })` – deckt normale API-Bodies mit
|
||||
eingebetteten Base64-Attachments ab, blockt aber DoS-Versuche mit 100MB-Payloads.
|
||||
|
||||
## Runde 2: Deep-Dive mit Audit-Agents (alle kritischen gefixt)
|
||||
|
||||
### 🔴 Kritisch
|
||||
|
||||
#### 9. Zip-Slip im Backup-Upload
|
||||
**Vorher:** `zip.extractAllTo(finalBackupDir, true)` in
|
||||
`backup.service.ts` extrahiert ZIP-Dateien ohne Validierung der Entry-Pfade.
|
||||
**Risiko:** Ein Angreifer lädt ein bösartiges ZIP hoch mit Entries wie
|
||||
`../../../etc/crontab` → Server-Filesystem-Overwrite, Root-Escalation möglich.
|
||||
**Fix:** ZIP-Entries werden jetzt einzeln durchlaufen. Jeder `entry.entryName`
|
||||
wird `path.resolve`-normalisiert und geprüft ob der Zielpfad innerhalb des
|
||||
Backup-Verzeichnisses bleibt. Absolute Pfade + Null-Bytes werden abgelehnt.
|
||||
|
||||
#### 10. Mass Assignment bei Customer/User
|
||||
**Vorher:** `updateCustomer`, `createCustomer`, `updateUser`, `createUser`
|
||||
haben `req.body` direkt oder via Spread an Prisma-`update/create` gereicht.
|
||||
**Risiko:**
|
||||
- Ein Angreifer mit `customers:update`-Permission konnte `portalPasswordHash`
|
||||
(bcrypt-Hash!), `portalPasswordResetToken`, `consentHash`, `customerNumber`
|
||||
direkt setzen
|
||||
- Bei User-Update: `roleIds: [1]` übergeben → **Privilege Escalation** zum Admin
|
||||
- `isActive: false` → andere User deaktivieren
|
||||
**Fix:** Neue Whitelist-Helper `pickCustomerUpdate/Create`, `pickUserUpdate/Create`
|
||||
in `utils/sanitize.ts`. Nur explizit erlaubte Felder gehen an Prisma durch.
|
||||
Kritische Felder wie `portalPasswordHash`, `customerNumber`, `id`, `createdAt`,
|
||||
`consentHash` sind **nicht** übernehmbar.
|
||||
|
||||
#### 11. IDOR bei weiteren sensiblen Endpoints
|
||||
Nach der ersten Runde kam der Agent auf **13 weitere IDOR-Stellen**:
|
||||
- `GET /meters/:meterId/readings` → fremde Zählerstände
|
||||
- `GET /emails/:emailId/attachments/*` → fremde Email-Anhänge
|
||||
- `GET /customers/:customerId/emails` → fremde Email-Inhalte (CachedEmail)
|
||||
- `GET /contracts/:contractId/emails` → fremde Email-Inhalte per Vertrag
|
||||
- `GET /emails/:id` → einzelne Email lesen
|
||||
- `GET /stressfrei-emails/:id` → leakt `emailPasswordEncrypted`
|
||||
- weitere…
|
||||
|
||||
**Fix:** `utils/accessControl.ts` deutlich ausgebaut um:
|
||||
- `canAccessAddress`
|
||||
- `canAccessBankCard`
|
||||
- `canAccessIdentityDocument`
|
||||
- `canAccessMeter`
|
||||
- `canAccessStressfreiEmail`
|
||||
- `canAccessCachedEmail`
|
||||
|
||||
Diese Helper laden die Ressource, prüfen die customerId und delegieren an
|
||||
`canAccessCustomer` (Portal-Isolation + Vollmachten). In allen kritischen
|
||||
Endpoints vor dem eigentlichen Datenzugriff aufgerufen.
|
||||
|
||||
Zusätzlich: `getEmail` (StressfreiEmail) filtert `emailPasswordEncrypted`
|
||||
für Portal-Kunden explizit raus, selbst wenn sie zufällig Zugriff haben.
|
||||
|
||||
### 🟡 Wichtig
|
||||
|
||||
#### 12. Path-Traversal bei Backup-Namen
|
||||
**Vorher:** `req.params.name` wurde direkt an `fs.readFile(path.join(backupDir, name))`
|
||||
weitergereicht. `../` würde aus dem Backup-Verzeichnis ausbrechen.
|
||||
**Fix:** Neuer `isValidBackupName()`-Guard: nur `[A-Za-z0-9_-]+`, kein `..`.
|
||||
|
||||
#### 13. Path-Traversal bei GDPR-Proof-Download
|
||||
**Vorher:** `path.join(uploads, request.proofDocument)` ohne Validation.
|
||||
Wenn ein Angreifer den `proofDocument`-Pfad in der DB manipulieren könnte
|
||||
(z.B. über Mass-Assignment – das haben wir aber oben gefixt), wäre arbitrary
|
||||
file download möglich.
|
||||
**Fix:** `path.resolve` auf den Pfad anwenden, prüfen dass das Ergebnis im
|
||||
uploads-Verzeichnis liegt.
|
||||
|
||||
---
|
||||
|
||||
## Nicht kritische Findings (Empfehlungen für später)
|
||||
|
||||
### 🟢 Token in Query-Parameter
|
||||
|
||||
Reference in New Issue
Block a user