From 4ab03404734450d7a3c5a3221257643cdb065035 Mon Sep 17 00:00:00 2001 From: duffyduck Date: Sun, 21 Jun 2026 15:24:57 +0200 Subject: [PATCH] =?UTF-8?q?Pentest=20R95:=20portalUsername=20(Manual-Modus?= =?UTF-8?q?)=20h=C3=A4rten?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit R95.1 MEDIUM: foo\r\nBcc:evil@x.de → Header-Injection-Vektor R95.3 LOW: @x.de → silent stripHtml-Mutation R95.4 LOW: >190 Zeichen → VARCHAR-Overflow → 500 statt 400 Fix: validatePortalUsername() in sanitize.ts mit Whitelist ^[A-Za-z0-9_\-/.@+ ]{0,100}$. Strukturell sind CRLF, Tab, alle Control-Chars, Tags und Quotes raus → R95.1+R95.3 ohne extra Check. Max 100 → ApiError(400) → R95.4. Raw-Input vor stripHtml geprüft (R87-Pattern). Eingehängt in sanitizeContractBody. R95.2 (Email-Format-Pflicht) bewusst NICHT übernommen: portalUsername ist im Manual-Modus nicht zwingend eine Email (Vodafone, 1&1, EWE und Stadtwerke nutzen Kundennummern oder Pseudonyme als Portal-Login). Doku in SECURITY-HARDENING.md § Runde 95. Frontend: maxLength={100} am Input als UX-Schicht. Co-Authored-By: Claude Opus 4.7 --- .../src/controllers/contract.controller.ts | 9 +++- backend/src/utils/sanitize.ts | 52 +++++++++++++++++++ docs/SECURITY-HARDENING.md | 40 ++++++++++++++ docs/todo.md | 17 ++++++ frontend/src/pages/contracts/ContractForm.tsx | 1 + 5 files changed, 118 insertions(+), 1 deletion(-) diff --git a/backend/src/controllers/contract.controller.ts b/backend/src/controllers/contract.controller.ts index b280a1f3..1817c8b2 100644 --- a/backend/src/controllers/contract.controller.ts +++ b/backend/src/controllers/contract.controller.ts @@ -8,7 +8,7 @@ import * as authorizationService from '../services/authorization.service.js'; import { recordPredecessorFinalReading } from '../services/customer.service.js'; import { ApiResponse, AuthRequest } from '../types/index.js'; import { logChange } from '../services/audit.service.js'; -import { sanitizeContract, sanitizeContractStrict, sanitizeContracts, sanitizeContractsStrict, stripHtml, sanitizeNotes, validateContractDocumentType, validateOptionalIsoDate, isContractIdentifierField, validateContractIdentifier } from '../utils/sanitize.js'; +import { sanitizeContract, sanitizeContractStrict, sanitizeContracts, sanitizeContractsStrict, stripHtml, sanitizeNotes, validateContractDocumentType, validateOptionalIsoDate, isContractIdentifierField, validateContractIdentifier, validatePortalUsername } from '../utils/sanitize.js'; import { ApiError } from '../utils/apiError.js'; import { canAccessContract } from '../utils/accessControl.js'; import { maybeActivateOnDeliveryConfirmation, withContractDocumentLock } from '../services/contractStatusScheduler.service.js'; @@ -52,6 +52,13 @@ function sanitizeContractBody(body: unknown, parentKey?: string): unknown { if (parentKey && isContractIdentifierField(parentKey)) { return validateContractIdentifier(body, parentKey); } + // Pentest 95.1/95.3/95.4 (LOW–MEDIUM, 2026-06-21): portalUsername + // (Manual-Modus) hatte gar keine Validierung – CRLF/Header-Injection, + // silent stripHtml-Mutation und VARCHAR-Overflow möglich. Gleiches + // Raw-Input-Pattern wie R87. + if (parentKey === 'portalUsername') { + return validatePortalUsername(body, parentKey); + } return stripHtml(body); } if (Array.isArray(body)) return body.map((v) => sanitizeContractBody(v, parentKey)); diff --git a/backend/src/utils/sanitize.ts b/backend/src/utils/sanitize.ts index fb3bc681..adaa6ff8 100644 --- a/backend/src/utils/sanitize.ts +++ b/backend/src/utils/sanitize.ts @@ -396,6 +396,58 @@ export function validateContractIdentifier( return trimmed; } +// Pentest 95.1/95.3/95.4 (MEDIUM/LOW, 2026-06-21): Manuelles +// `portalUsername` am Vertrag hatte gar keine Validierung. Drei +// nachweisbare Effekte: +// - `foo\r\nBcc:evil@x.de` (CRLF) verbatim gespeichert → +// Header-Injection-Vektor sobald der Wert in Mail-Templates +// oder PDF-Footers landet. +// - `@x.de` lief durch stripHtml → +// stille Mutation (R87.1/R89.2-Pattern auf neuem Feld). +// - >190 Zeichen → VARCHAR-Overflow → generischer 500 statt 400. +// +// Bewusst NICHT übernommen wurde R95.2 (Email-Format-Pflicht): +// `portalUsername` ist im Manual-Modus nicht zwingend eine +// E-Mail. Vodafone, 1&1, EWE und etliche Stadtwerke nutzen +// Kundennummern, Pseudonyme oder Customer-IDs als Portal-Login. +// Eine `email().regex()`-Pflicht würde legitime Logins ablehnen. +// Der Stressfrei-Modus hängt eh an einer schon validierten +// Email-Stammdate (assertValidForwardingEmail). +// +// Allowed: Alphanumerisch + `_`, `-`, `.`, `/`, `@`, `+`, Space. +// Damit sind Vodafone-Kunden-IDs (`12345678`), Pseudonyme +// (`max.mustermann`), Plus-Tag-Emails (`m+tag@example.com`) +// und gemischte Formen abgedeckt. Strukturell sind CRLF, Tab, +// alle Control-Chars, Tags und Quotes raus → R95.1+R95.3 ohne +// extra Check. Max 100 Zeichen << VARCHAR(191) → R95.4. +const PORTAL_USERNAME_ALLOWED = /^[A-Za-z0-9_\-/.@+ ]{0,100}$/; +const PORTAL_USERNAME_MAX_LEN = 100; + +export function validatePortalUsername( + raw: unknown, + fieldLabel = 'portalUsername', +): string | null { + if (raw == null) return null; + if (typeof raw !== 'string') { + throw new ApiError(400, `${fieldLabel} muss ein Text sein.`); + } + const trimmed = raw.trim(); + if (trimmed === '') return null; + if (trimmed.length > PORTAL_USERNAME_MAX_LEN) { + throw new ApiError( + 400, + `${fieldLabel} darf maximal ${PORTAL_USERNAME_MAX_LEN} Zeichen lang sein.`, + ); + } + if (!PORTAL_USERNAME_ALLOWED.test(trimmed)) { + throw new ApiError( + 400, + `${fieldLabel} enthält unzulässige Zeichen (erlaubt: Buchstaben, Ziffern, Punkt, Bindestrich, Schrägstrich, Unterstrich, @, +, Leerzeichen).`, + ); + } + return trimmed; +} + // Pentest 89.1 + 89.2 (MEDIUM/LOW, 2026-06-21): Postadressen am // Provider (`contactAddress`, `cancellationAddress`). sanitizeNotes // hat das Length-Cap silent durchgeschoben (slice statt Error) und diff --git a/docs/SECURITY-HARDENING.md b/docs/SECURITY-HARDENING.md index f0db7ce1..9e293592 100644 --- a/docs/SECURITY-HARDENING.md +++ b/docs/SECURITY-HARDENING.md @@ -614,6 +614,46 @@ einer mehrzeiligen Postadresse. --- +## 🔒 Runde 95 – Portal-Username-Validierung + +**Findings (R95.1 MEDIUM + R95.3 LOW + R95.4 LOW):** + +`portalUsername` (Manual-Input-Modus am Vertrag) hatte gar keine +Validierung. Drei nachweisbare Effekte: + +- **R95.1**: `foo\r\nBcc:evil@x.de` (CRLF) wurde verbatim + gespeichert → Header-Injection-Vektor sobald der Wert in + Mail-Templates oder PDF-Footers landet. +- **R95.3**: `@x.de` lief durch + `stripHtml` → stille Mutation zu `@x.de` (R87.1/R89.2-Pattern + auf neuem Feld). +- **R95.4**: >190 Zeichen → VARCHAR-Overflow → generischer 500 + statt sauberem 400. + +**Fix:** `validatePortalUsername(raw, fieldLabel)` in +[`backend/src/utils/sanitize.ts`](../backend/src/utils/sanitize.ts): + +- Whitelist `^[A-Za-z0-9_\-/.@+ ]{0,100}$`. Strukturell sind + CRLF, Tab, alle Control-Chars, Tags (`<`, `>`) und Quotes raus + → R95.1 und R95.3 ohne extra Check. +- Max 100 Zeichen → `ApiError(400, …)` → R95.4 mit klarer Meldung. +- Raw-Input direkt validiert (kein `stripHtml` davor) – gleicher + R87-Pattern wie bei Contract-Identifier und Provider-Address. +- Eingehängt in `sanitizeContractBody` als eigener Branch. + +**Bewusst NICHT übernommen: R95.2 (Email-Format-Pflicht).** + +Der Pentester schlägt `z.string().email()` vor, weil der „Kunde +sich sonst nicht einloggen kann". Falsche Annahme: `portalUsername` +ist im Manual-Modus **nicht zwingend eine E-Mail**. Vodafone, 1&1, +EWE und etliche Stadtwerke nutzen reine Kundennummern (`12345678`), +Pseudonyme (`max.mustermann`) oder Customer-IDs als Portal-Login. +Eine Email-Pflicht würde legitime Logins ablehnen. Der Stressfrei- +Modus hängt sowieso an einer schon validierten Email-Stammdate +(`assertValidForwardingEmail`). + +--- + ## 🧭 Wann ist „dicht" dicht? 100 % gibt es nicht. Erreicht ist: diff --git a/docs/todo.md b/docs/todo.md index e617da45..961e29ea 100644 --- a/docs/todo.md +++ b/docs/todo.md @@ -97,6 +97,23 @@ isolierte Instanz (keine Multi-Tenancy im Code), Provisioning + Abrechnung ## ✅ Erledigt +- [x] **🔒 Pentest R95 – Portal-Username (Manual-Modus) härten** + - R95.1 (MEDIUM): `foo\r\nBcc:evil@x.de` → Header-Injection-Vektor + sobald der Wert in Mail-Templates / PDF-Footer landet. + - R95.3 (LOW): `@x.de` → silent stripHtml-Mutation + (R87.1-Pattern, dritter Treffer auf demselben Bug). + - R95.4 (LOW): >190 Zeichen → VARCHAR-Overflow → 500 statt 400. + - Fix: `validatePortalUsername()` in `sanitize.ts` mit Whitelist + `^[A-Za-z0-9_\-/.@+ ]{0,100}$`. Strukturell sind CRLF, Tab, alle + Control-Chars, Tags und Quotes raus → R95.1+R95.3 ohne extra + Check. Max 100 → `ApiError(400)` → R95.4 sauber. Raw-Input direkt + validiert (R87-Pattern). Eingehängt in `sanitizeContractBody`. + - Frontend: `maxLength={100}` am Input. + - **R95.2 bewusst nicht übernommen** (Email-Format-Pflicht): das + Feld ist im Manual-Modus nicht zwingend eine E-Mail – Vodafone, + 1&1, EWE und Stadtwerke nutzen Kundennummern oder Pseudonyme als + Portal-Login. Doku in `SECURITY-HARDENING.md § Runde 95`. + - [x] **🔒 Pentest R93 – Leerer String != fehlender Param** - R93.1 (INFO): `?accountId=` (explizit-leer) wurde wie `?accountId` weggelassen behandelt → 200 statt 400 auf optionalen Endpunkten. diff --git a/frontend/src/pages/contracts/ContractForm.tsx b/frontend/src/pages/contracts/ContractForm.tsx index ef1b2d9d..53441bdb 100644 --- a/frontend/src/pages/contracts/ContractForm.tsx +++ b/frontend/src/pages/contracts/ContractForm.tsx @@ -1053,6 +1053,7 @@ export default function ContractForm() { {usernameType === 'manual' && ( )}