Pentest R87: Identifier-Whitelist vor stripHtml ziehen
R87.1 LOW: stripHtml lief im R86-Fix VOR der Whitelist. `<b>bold</b>` ging als `"bold"` mit 200 OK durch, `<script>…</script>` reduzierte auf leeren String → null in DB → vorheriger Wert ohne Fehlermeldung überschrieben. Fix: validateContractIdentifier läuft jetzt direkt gegen den Raw-Input für die fünf Identifier-Felder. Die strikte Whitelist lehnt eh alles ab, was stripHtml normalerweise auffangen würde (Tags, Schemes, Zero-Width, Homoglyphe, Percent-Encoding) – Defense-in-Depth bleibt, nur ehrlich (400 statt silent-200). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
@@ -36,14 +36,23 @@ function sanitizeContractBody(body: unknown, parentKey?: string): unknown {
|
||||
if (body === null || body === undefined) return body;
|
||||
if (typeof body === 'string') {
|
||||
if (parentKey && PASSTHROUGH_KEYS.has(parentKey)) return body;
|
||||
const stripped = stripHtml(body);
|
||||
// Pentest 86.1/86.2 (LOW, 2026-06-19): Längen- + Whitelist-Check auf
|
||||
// Kunden-/Vertrags-/Auftragsnummer-Feldern. validateContractIdentifier
|
||||
// wirft ApiError(400) bei Verstoß → saubere 400-Antwort statt 500.
|
||||
//
|
||||
// Pentest 87.1 (LOW, 2026-06-19): Identifier-Felder MÜSSEN gegen den
|
||||
// Raw-Input geprüft werden, NICHT gegen den stripHtml-Output. Sonst
|
||||
// verschluckt der Sanitizer Tag-Verstöße still: `<b>bold</b>` würde
|
||||
// als `"bold"` mit 200 OK durchgehen, `<script>alert(1)</script>`
|
||||
// sogar zu `null` und damit den vorherigen Wert überschreiben.
|
||||
// Die strikte Whitelist (`^[A-Za-z0-9_\-/. ]{0,100}$`) deckt alle
|
||||
// Bypässe ab, die stripHtml normalerweise auffangen würde
|
||||
// (Tags, Schemes, Zero-Width-Chars, Homoglyphe, Percent-Encoding) –
|
||||
// sie sind alle nicht in der Allowlist und fliegen mit 400 raus.
|
||||
if (parentKey && isContractIdentifierField(parentKey)) {
|
||||
return validateContractIdentifier(stripped, parentKey);
|
||||
return validateContractIdentifier(body, parentKey);
|
||||
}
|
||||
return stripped;
|
||||
return stripHtml(body);
|
||||
}
|
||||
if (Array.isArray(body)) return body.map((v) => sanitizeContractBody(v, parentKey));
|
||||
if (typeof body === 'object') {
|
||||
|
||||
@@ -537,6 +537,44 @@ erneut als „offenes Finding" auftaucht.
|
||||
|
||||
---
|
||||
|
||||
## 🔒 Runde 87 – Whitelist vor Sanitizer (silent-mutation-Schutz)
|
||||
|
||||
**Finding (LOW): Sanitizer-Order maskiert Tag-Verstöße**
|
||||
|
||||
Im ursprünglichen R86-Fix lief `stripHtml(body)` **vor**
|
||||
`validateContractIdentifier`. Das hatte einen subtilen Bypass:
|
||||
|
||||
| Payload | Status | Tatsächlich gespeichert |
|
||||
|--------------------------------------|------------|-------------------------|
|
||||
| `<b>bold</b>` | 200 OK | `"bold"` (silent strip) |
|
||||
| `EVN<b>2024</b>` | 200 OK | `"EVN2024"` |
|
||||
| `<script>alert(1)</script>` | **200 OK** | `null` – **vorherigen Wert überschrieben** |
|
||||
| `foo<bar>baz` | 200 OK | `"foobarbaz"` |
|
||||
|
||||
Kein direkter XSS-Vektor (React + DB-Whitelist greifen weiterhin),
|
||||
aber zwei reale UX-/Datenintegritäts-Risiken:
|
||||
|
||||
1. Admin tippt `VG<2024>001`, bekommt 200 zurück, gespeichert ist
|
||||
`VG2024001` ohne Hinweis auf die Mutation.
|
||||
2. Werte die komplett aus Tags bestehen (`<script>…</script>`)
|
||||
werden vom Sanitizer auf den leeren String reduziert →
|
||||
`null` in der DB → **vorheriger Wert wird stillschweigend
|
||||
gelöscht**.
|
||||
|
||||
**Fix:** Validierungs-Reihenfolge für die fünf Identifier-Felder
|
||||
umgedreht – `validateContractIdentifier` läuft jetzt **direkt
|
||||
gegen den Raw-Input**, ohne dass `stripHtml` ihn vorher
|
||||
glättet. Die strikte Whitelist
|
||||
`^[A-Za-z0-9_\-/. ]{0,100}$` lehnt sowieso alles ab, was
|
||||
`stripHtml` normalerweise abgefangen hätte (Tags, Schemes,
|
||||
Zero-Width-Chars, Homoglyphe, Percent-Encoding) – Defense-in-
|
||||
Depth bleibt unverändert, nur jetzt ehrlich (400 statt silent-200).
|
||||
|
||||
Single-Line-Patch in [`backend/src/controllers/contract.controller.ts`](../backend/src/controllers/contract.controller.ts)
|
||||
`sanitizeContractBody`.
|
||||
|
||||
---
|
||||
|
||||
## 🧭 Wann ist „dicht" dicht?
|
||||
|
||||
100 % gibt es nicht. Erreicht ist:
|
||||
|
||||
@@ -97,6 +97,20 @@ isolierte Instanz (keine Multi-Tenancy im Code), Provisioning + Abrechnung
|
||||
|
||||
## ✅ Erledigt
|
||||
|
||||
- [x] **🔒 Pentest R87 – Whitelist vor Sanitizer (silent-mutation-Schutz)**
|
||||
- R87.1 (LOW): `stripHtml` lief im R86-Fix VOR der Whitelist.
|
||||
Tags wurden still weggestrippt → 200 OK mit mutierten Werten,
|
||||
`<script>…</script>` reduzierte auf leeren String → `null` in
|
||||
der DB → vorheriger Wert ohne Fehlermeldung überschrieben.
|
||||
- Fix: Validierungs-Reihenfolge für die fünf Identifier-Felder
|
||||
umgedreht – `validateContractIdentifier` läuft jetzt direkt
|
||||
gegen den Raw-Input. Die strikte Whitelist lehnt eh alles
|
||||
ab, was stripHtml normalerweise auffangen würde (Tags,
|
||||
Schemes, Zero-Width, Homoglyphe, Percent-Encoding) – Defense-
|
||||
in-Depth bleibt, nur ehrlich (400 statt silent-200).
|
||||
- Single-Line-Patch in `contract.controller.ts`, Doku in
|
||||
`SECURITY-HARDENING.md § Runde 87`.
|
||||
|
||||
- [x] **🔒 Pentest R86 – Vertrags-Identifier härten**
|
||||
- R86.1 (LOW): >999-Zeichen-Strings auf Kunden-/Vertrags-/
|
||||
Auftragsnummer warfen 500 (DB-Overflow `VARCHAR(191)`) statt 400.
|
||||
|
||||
Reference in New Issue
Block a user