232 lines
11 KiB
Markdown
232 lines
11 KiB
Markdown
# Security-Review vor 1.0.0
|
||
|
||
> 📌 **Diese Datei dokumentiert nur die ersten 2 Runden ausführlich.**
|
||
> Die vollständige Hardening-Story über alle **8 Runden** inkl. Live-Test-
|
||
> Tabellen findest du in **[SECURITY-HARDENING.md](./SECURITY-HARDENING.md)**.
|
||
|
||
> **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).
|
||
|
||
## Gefundene Probleme & Fixes
|
||
|
||
### 🔴 KRITISCH (sofort gefixt)
|
||
|
||
#### 1. CORS komplett offen
|
||
**Vorher:** `app.use(cors())` – jede Origin darf Requests senden.
|
||
**Risiko:** Fremde Websites können bei eingeloggtem User Requests mit dessen
|
||
JWT durchführen (wenn Token in Cookies wäre – bei localStorage weniger relevant,
|
||
aber trotzdem schlechte Praxis).
|
||
**Fix:** CORS nur für explizit konfigurierte Origins (via `CORS_ORIGINS` ENV),
|
||
in Production per Default komplett aus (Frontend läuft unter gleicher Origin).
|
||
|
||
#### 2. Keine Security-Headers (Helmet fehlt)
|
||
**Vorher:** Keine HTTP-Security-Headers gesetzt.
|
||
**Risiko:** XSS, Clickjacking, MIME-Sniffing, Missing HSTS.
|
||
**Fix:** `helmet`-Middleware aktiviert – setzt automatisch:
|
||
X-Frame-Options, X-Content-Type-Options, Referrer-Policy, HSTS (in HTTPS),
|
||
Cross-Origin-Resource-Policy.
|
||
|
||
#### 3. JWT-Fallback-Secret
|
||
**Vorher:** `jwt.verify(token, process.env.JWT_SECRET || 'fallback-secret')`
|
||
**Risiko:** Wenn `.env` kaputt ist oder Secret leer → bekannter String
|
||
"fallback-secret" → **Tokens können gefälscht werden!**
|
||
**Fix:** Beim Server-Start wird geprüft, dass JWT_SECRET mindestens 32 Zeichen lang
|
||
und ENCRYPTION_KEY exakt 64 Hex-Zeichen hat. Sonst Abbruch mit klarer Fehlermeldung.
|
||
Fallback wurde aus dem Code entfernt.
|
||
|
||
#### 4. IDOR bei sensiblen Contract-Endpoints
|
||
**Vorher:** Portal-Kunden haben `contracts:read` Permission → können über
|
||
geratene IDs auf **fremde** Daten zugreifen:
|
||
- `GET /contracts/:id/password` → Passwort im Klartext
|
||
- `GET /contracts/simcard/:id/credentials` → PIN/PUK
|
||
- `GET /contracts/:id/internet-credentials` → Internet-Passwort
|
||
- `GET /contracts/phonenumber/:id/sip-credentials` → SIP-Passwort
|
||
- `GET /contracts/:id/documents` → Vertragsdokumente
|
||
- `GET /contracts/:id/invoices` → Rechnungen
|
||
- `POST /contracts/:id/invoices` → Rechnung zu fremdem Vertrag hinzufügen
|
||
**Fix:** Neuer Helper `canAccessContract()` in `backend/src/utils/accessControl.ts`.
|
||
Wird in allen sensiblen Endpoints aufgerufen und prüft:
|
||
- Mitarbeiter/Admin → OK
|
||
- Portal-Kunde + eigener Vertrag → OK
|
||
- Portal-Kunde + vertretener Kunde MIT gültiger Vollmacht → OK
|
||
- Sonst 403 Forbidden
|
||
|
||
#### 5. XSS via Email-Body
|
||
**Vorher:** `<div dangerouslySetInnerHTML={{ __html: email.htmlBody }} />`
|
||
**Risiko:** Ein Angreifer sendet Mail mit `<script>fetch('/api/...')` →
|
||
wird im Browser des Mitarbeiters ausgeführt → JWT-Token-Diebstahl möglich.
|
||
**Fix:** DOMPurify sanitized `htmlBody` vor dem Rendern:
|
||
- Verbietet: script, style, iframe, object, embed, form, inline-handler
|
||
- Erlaubt: normale Formatierung, Bilder, Links
|
||
- Zusätzlich: target=_blank damit Links neue Tabs öffnen
|
||
|
||
#### 6. Customer-API leakt Passwort-Hashes + Reset-Tokens
|
||
**Vorher:** `getCustomer` / `getCustomers` gab alle Felder zurück inklusive:
|
||
- `portalPasswordHash` (bcrypt)
|
||
- `portalPasswordEncrypted` (symmetrisch, entschlüsselbar mit Key)
|
||
- `portalPasswordResetToken` (gültig 2h, damit könnte man das Passwort zurücksetzen)
|
||
**Fix:** Zentrale Sanitizer-Helper in `backend/src/utils/sanitize.ts`:
|
||
- `sanitizeCustomer` → entfernt Hash + Reset-Token
|
||
- `sanitizeCustomerStrict` → zusätzlich ohne Encrypted-Passwort
|
||
(für Nicht-Admin-Rollen)
|
||
- Im `getCustomer`/`getCustomers` angewendet: Admins sehen encrypted
|
||
(um Passwort in UI anzeigen zu können), alle anderen nicht.
|
||
|
||
### 🟡 WICHTIG (gefixt)
|
||
|
||
#### 7. Portal-JWT-Invalidation fehlte
|
||
**Vorher:** Nach einem Portal-Passwort-Reset blieben alte JWTs bis zum Ablauf (7d) gültig.
|
||
**Risiko:** Wenn ein Angreifer einen Token geklaut hat, konnte der Kunde das
|
||
Passwort zwar ändern, aber der Angreifer blieb eingeloggt.
|
||
**Fix:** Neues Feld `Customer.portalTokenInvalidatedAt` analog zu
|
||
`User.tokenInvalidatedAt`. Wird bei Portal-Passwort-Reset auf `now()` gesetzt.
|
||
Auth-Middleware prüft bei Portal-Sessions diesen Timestamp gegen `token.iat`.
|
||
|
||
#### 8. express.json() ohne Size-Limit
|
||
**Vorher:** Default 100KB – aber unklar und nicht explizit.
|
||
**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
|
||
Für Attachment-Downloads/iframes wird das JWT als `?token=...` mitgegeben.
|
||
**Risiko:** Token landet in Server-Access-Logs, Browser-History, Referer-Headers.
|
||
**Mitigation aktuell:** JWT läuft nach 7d ab, und bei `password-reset` werden
|
||
alle Sessions gekickt.
|
||
**Bessere Lösung (später):** Kurzlebige Download-Tokens (5 Min) statt JWT direkt.
|
||
|
||
### 🟢 Upload: nur Browser-MIME-Check
|
||
Multer prüft nur den vom Browser gesendeten Content-Type. Ein Angreifer könnte
|
||
eine Shell mit `application/pdf` hochladen.
|
||
**Mitigation aktuell:**
|
||
- Uploads-Ordner hat keine Execute-Rechte (Linux-Standard)
|
||
- Dateien werden mit uniquem Namen + Original-Extension gespeichert
|
||
- Apache/Caddy served Uploads mit `Content-Disposition: attachment` inline (keine Ausführung)
|
||
**Besser (später):** Magic-Byte-Check via `file-type` npm-Paket.
|
||
|
||
### 🟢 `.env` in git history
|
||
Die initiale `.env` mit Demo-Secrets ist im ersten Commit eingecheckt.
|
||
**Risiko:** Wenn das Repo öffentlich wird, sind die Demo-Keys bekannt.
|
||
**Action:** Vor Öffentlich-Machen: `openssl rand -hex 64` für neuen JWT_SECRET
|
||
und `openssl rand -hex 32` für neuen ENCRYPTION_KEY in `.env.production`.
|
||
Optional: `git filter-repo` um `.env` aus History zu löschen.
|
||
|
||
## Deployment-Checkliste vor Go-Live
|
||
|
||
- [ ] **ENV-Vars setzen:**
|
||
- `JWT_SECRET` neu generiert (`openssl rand -hex 64`)
|
||
- `ENCRYPTION_KEY` neu generiert (`openssl rand -hex 32`)
|
||
- `NODE_ENV=production`
|
||
- `CORS_ORIGINS=https://crm.meinedomain.de` (oder leer wenn SPA unter gleicher Origin)
|
||
- `PUBLIC_URL=https://crm.meinedomain.de` (für Reset-Links in E-Mails)
|
||
- [ ] **Helmet HSTS aktiv** (automatisch mit helmet + HTTPS hinter Caddy)
|
||
- [ ] **Dependencies aktuell:** `npm audit fix` lauen lassen
|
||
- [ ] **DB-User minimal:** Prod-User darf nur INSERT/UPDATE/DELETE/SELECT auf opencrm DB,
|
||
nicht DROP/ALTER/CREATE
|
||
- [ ] **Uploads-Ordner:** chmod 750, keine Execute-Rechte
|
||
- [ ] **Backup-Job:** Crontab mit täglichem `npm run db:backup`
|
||
- [ ] **Log-Rotation:** logrotate für Node-Process-Logs
|
||
- [ ] **Monitoring:** uptime-kuma o.Ä. auf `/api/health`
|
||
- [ ] **Reverse-Proxy (Caddy) setzt:**
|
||
- HSTS (mindestens 1 Jahr)
|
||
- automatisches SSL via Let's Encrypt
|
||
- Body-Size-Limit (Caddy-Config)
|
||
|
||
## Was getestet werden MUSS (vor öffentlichem Deployment)
|
||
|
||
1. **IDOR-Tests:** Als Portal-Kunde A einloggen, fremde IDs per URL/API probieren
|
||
→ alle müssen 403 geben (siehe TESTING.md)
|
||
2. **XSS-Tests:** Test-Mail mit `<script>alert(1)</script>` in HTML-Body senden,
|
||
im Email-Client öffnen → kein Alert
|
||
3. **Rate-Limit-Tests:** 11x falsch einloggen → muss blocken
|
||
4. **Password-Reset-Tests:** Reset-Link 2x nutzen → zweites Mal fehlschlägt
|
||
|
||
## Übersicht der Code-Änderungen
|
||
|
||
| Datei | Änderung |
|
||
|---|---|
|
||
| `backend/src/index.ts` | Helmet, CORS-Config, Body-Limit, ENV-Check beim Start |
|
||
| `backend/src/middleware/auth.ts` | JWT-Fallback raus, Portal-Token-Invalidation |
|
||
| `backend/src/services/auth.service.ts` | JWT-Fallback raus, `portalTokenInvalidatedAt` setzen |
|
||
| `backend/src/utils/accessControl.ts` | **NEU** – `canAccessContract`, `canAccessCustomer` |
|
||
| `backend/src/utils/sanitize.ts` | **NEU** – Sanitizer für Customer/User |
|
||
| `backend/src/controllers/contract.controller.ts` | IDOR-Schutz in 5 Endpoints |
|
||
| `backend/src/controllers/invoice.controller.ts` | IDOR-Schutz in 2 Endpoints |
|
||
| `backend/src/controllers/customer.controller.ts` | Sanitizer in getCustomer/getCustomers |
|
||
| `backend/prisma/schema.prisma` | `Customer.portalTokenInvalidatedAt` |
|
||
| `frontend/src/components/email/EmailDetail.tsx` | DOMPurify für htmlBody |
|