Pentest R86: Vertrags-Identifier max 100 + Charset-Whitelist
R86.1 LOW + R86.2 LOW: >999-Zeichen liefen in DB-Overflow (500
statt 400), Attribut-Injection (`foo" onerror=…` ohne
umschließenden Tag) überlebte stripHtml.
Fix: validateContractIdentifier() (max 100,
^[A-Za-z0-9_\-/. ]{0,100}$) in sanitize.ts, eingehängt in
sanitizeContractBody. Wirft ApiError(400, …). Literales Space
statt \s → kein CRLF/Tab → kein Header-Injection-Vektor in
CSV-/Mail-/PDF-Export. Greift auf alle fünf Identifier-Felder
(Provider + Sales-Platform). ContractForm-Inputs bekommen
maxLength={100} als UX-Schicht.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
@@ -8,7 +8,7 @@ import * as authorizationService from '../services/authorization.service.js';
|
|||||||
import { recordPredecessorFinalReading } from '../services/customer.service.js';
|
import { recordPredecessorFinalReading } from '../services/customer.service.js';
|
||||||
import { ApiResponse, AuthRequest } from '../types/index.js';
|
import { ApiResponse, AuthRequest } from '../types/index.js';
|
||||||
import { logChange } from '../services/audit.service.js';
|
import { logChange } from '../services/audit.service.js';
|
||||||
import { sanitizeContract, sanitizeContractStrict, sanitizeContracts, sanitizeContractsStrict, stripHtml, sanitizeNotes, validateContractDocumentType, validateOptionalIsoDate } from '../utils/sanitize.js';
|
import { sanitizeContract, sanitizeContractStrict, sanitizeContracts, sanitizeContractsStrict, stripHtml, sanitizeNotes, validateContractDocumentType, validateOptionalIsoDate, isContractIdentifierField, validateContractIdentifier } from '../utils/sanitize.js';
|
||||||
import { ApiError } from '../utils/apiError.js';
|
import { ApiError } from '../utils/apiError.js';
|
||||||
import { canAccessContract } from '../utils/accessControl.js';
|
import { canAccessContract } from '../utils/accessControl.js';
|
||||||
import { maybeActivateOnDeliveryConfirmation, withContractDocumentLock } from '../services/contractStatusScheduler.service.js';
|
import { maybeActivateOnDeliveryConfirmation, withContractDocumentLock } from '../services/contractStatusScheduler.service.js';
|
||||||
@@ -36,7 +36,14 @@ function sanitizeContractBody(body: unknown, parentKey?: string): unknown {
|
|||||||
if (body === null || body === undefined) return body;
|
if (body === null || body === undefined) return body;
|
||||||
if (typeof body === 'string') {
|
if (typeof body === 'string') {
|
||||||
if (parentKey && PASSTHROUGH_KEYS.has(parentKey)) return body;
|
if (parentKey && PASSTHROUGH_KEYS.has(parentKey)) return body;
|
||||||
return stripHtml(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.
|
||||||
|
if (parentKey && isContractIdentifierField(parentKey)) {
|
||||||
|
return validateContractIdentifier(stripped, parentKey);
|
||||||
|
}
|
||||||
|
return stripped;
|
||||||
}
|
}
|
||||||
if (Array.isArray(body)) return body.map((v) => sanitizeContractBody(v, parentKey));
|
if (Array.isArray(body)) return body.map((v) => sanitizeContractBody(v, parentKey));
|
||||||
if (typeof body === 'object') {
|
if (typeof body === 'object') {
|
||||||
|
|||||||
@@ -343,6 +343,59 @@ export function validateOptionalIsoDate(raw: unknown, fieldLabel: string): strin
|
|||||||
return trimmed;
|
return trimmed;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Pentest 86.1 + 86.2 (LOW, 2026-06-19): Kunden-/Vertrags-/Auftrags-
|
||||||
|
// Nummern bei Anbieter und Vertriebsplattform hatten keine Längen- oder
|
||||||
|
// Zeichen-Validierung. >1000 Zeichen-Strings warfen einen generischen
|
||||||
|
// 500er (DB-Overflow VARCHAR(191)) statt eines 400ers. Außerdem
|
||||||
|
// überlebten Attribut-Injection-Payloads wie `foo" onerror="alert(1)`
|
||||||
|
// die stripHtml-Defense (kein umschließender Tag → kein Match), die
|
||||||
|
// in PDF-/Mail-Export potentiell aktiv werden könnten.
|
||||||
|
//
|
||||||
|
// Whitelist orientiert sich am Vorschlag des Pentesters
|
||||||
|
// `^[\w\-\/\s\.]*$` – Whitespace ist hier bewusst NUR ein literales
|
||||||
|
// Space, NICHT `\s` (kein CRLF/Tab → kein Header-Injection-Vektor
|
||||||
|
// in CSV-/Mail-Exporten). Max 100 Zeichen reicht für jede reale
|
||||||
|
// Kunden-/Vertrags-Nummer und bleibt deutlich unter dem VARCHAR(191)-
|
||||||
|
// Limit der DB-Spalte.
|
||||||
|
const CONTRACT_IDENTIFIER_FIELDS: ReadonlySet<string> = new Set([
|
||||||
|
'customerNumberAtProvider',
|
||||||
|
'contractNumberAtProvider',
|
||||||
|
'orderNumberAtSalesPlatform',
|
||||||
|
'customerNumberAtSalesPlatform',
|
||||||
|
'contractNumberAtSalesPlatform',
|
||||||
|
]);
|
||||||
|
const CONTRACT_IDENTIFIER_ALLOWED = /^[A-Za-z0-9_\-/. ]{0,100}$/;
|
||||||
|
const CONTRACT_IDENTIFIER_MAX_LEN = 100;
|
||||||
|
|
||||||
|
export function isContractIdentifierField(key: string): boolean {
|
||||||
|
return CONTRACT_IDENTIFIER_FIELDS.has(key);
|
||||||
|
}
|
||||||
|
|
||||||
|
export function validateContractIdentifier(
|
||||||
|
raw: unknown,
|
||||||
|
fieldLabel: string,
|
||||||
|
): 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 > CONTRACT_IDENTIFIER_MAX_LEN) {
|
||||||
|
throw new ApiError(
|
||||||
|
400,
|
||||||
|
`${fieldLabel} darf maximal ${CONTRACT_IDENTIFIER_MAX_LEN} Zeichen lang sein.`,
|
||||||
|
);
|
||||||
|
}
|
||||||
|
if (!CONTRACT_IDENTIFIER_ALLOWED.test(trimmed)) {
|
||||||
|
throw new ApiError(
|
||||||
|
400,
|
||||||
|
`${fieldLabel} enthält unzulässige Zeichen (erlaubt: Buchstaben, Ziffern, Punkt, Bindestrich, Schrägstrich, Unterstrich, Leerzeichen).`,
|
||||||
|
);
|
||||||
|
}
|
||||||
|
return trimmed;
|
||||||
|
}
|
||||||
|
|
||||||
const NOTES_DEFAULT_MAX = 2000;
|
const NOTES_DEFAULT_MAX = 2000;
|
||||||
export function sanitizeNotes(raw: unknown, maxLength: number = NOTES_DEFAULT_MAX): string | null {
|
export function sanitizeNotes(raw: unknown, maxLength: number = NOTES_DEFAULT_MAX): string | null {
|
||||||
if (raw == null) return null;
|
if (raw == null) return null;
|
||||||
|
|||||||
@@ -504,6 +504,39 @@ erneut als „offenes Finding" auftaucht.
|
|||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
|
## 🔒 Runde 86 – Vertrags-Identifier-Validierung
|
||||||
|
|
||||||
|
**Findings (beide LOW):**
|
||||||
|
|
||||||
|
- **R86.1**: Strings >999 Zeichen in `orderNumberAtSalesPlatform` / den
|
||||||
|
vier verwandten Sales-/Provider-Nummern-Feldern endeten mit
|
||||||
|
generischem 500 (DB-Overflow `VARCHAR(191)`) statt sauberem 400.
|
||||||
|
- **R86.2**: Attribut-Injection-Payload `foo" onerror="alert(1)`
|
||||||
|
(kein umschließender Tag) überlebte `stripHtml`. React escaped
|
||||||
|
Attribute, aber sobald der Wert in PDF-/Mail-/CSV-Export fließt,
|
||||||
|
ist es potentiell aktiv.
|
||||||
|
|
||||||
|
**Fix:** `validateContractIdentifier(raw, fieldLabel)` in
|
||||||
|
[`backend/src/utils/sanitize.ts`](../backend/src/utils/sanitize.ts):
|
||||||
|
|
||||||
|
- Max-Länge 100 Zeichen (deutlich unter VARCHAR(191)).
|
||||||
|
- Whitelist `^[A-Za-z0-9_\-/. ]{0,100}$` – Buchstaben, Ziffern,
|
||||||
|
Punkt, Bindestrich, Schrägstrich, Unterstrich und literales
|
||||||
|
Leerzeichen. Bewusst NICHT `\s` (kein CRLF/Tab → kein
|
||||||
|
Header-Injection-Vektor in CSV-/Mail-Exporten).
|
||||||
|
- Bei Verstoß: `ApiError(400, …)` mit konkreter Fehlermeldung
|
||||||
|
statt 500.
|
||||||
|
- Eingehängt in `sanitizeContractBody` → läuft automatisch für alle
|
||||||
|
fünf Identifier-Felder (`customerNumberAtProvider`,
|
||||||
|
`contractNumberAtProvider`, `orderNumberAtSalesPlatform`,
|
||||||
|
`customerNumberAtSalesPlatform`, `contractNumberAtSalesPlatform`)
|
||||||
|
bei jedem Create/Update.
|
||||||
|
- Frontend: `maxLength={100}` als zusätzliche UX-Schicht im
|
||||||
|
ContractForm – Server-seitige Validierung bleibt die einzige
|
||||||
|
Wahrheit, das HTML-Attribut spart nur den unnötigen Round-Trip.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
## 🧭 Wann ist „dicht" dicht?
|
## 🧭 Wann ist „dicht" dicht?
|
||||||
|
|
||||||
100 % gibt es nicht. Erreicht ist:
|
100 % gibt es nicht. Erreicht ist:
|
||||||
|
|||||||
@@ -97,6 +97,21 @@ isolierte Instanz (keine Multi-Tenancy im Code), Provisioning + Abrechnung
|
|||||||
|
|
||||||
## ✅ Erledigt
|
## ✅ Erledigt
|
||||||
|
|
||||||
|
- [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.
|
||||||
|
- R86.2 (LOW/INFO): Attribut-Injection ohne umschließenden Tag
|
||||||
|
(`foo" onerror=…`) überlebte `stripHtml` – kein Risiko in der React-
|
||||||
|
UI, aber relevant für PDF/Mail/CSV-Export.
|
||||||
|
- Fix: zentraler `validateContractIdentifier()` in `sanitize.ts`
|
||||||
|
mit Max-100 und Whitelist `^[A-Za-z0-9_\-/. ]{0,100}$`. Bewusst
|
||||||
|
literales Space statt `\s`, damit kein CRLF/Tab passiert (Header-
|
||||||
|
Injection). Wirft `ApiError(400, …)` mit klarer Meldung.
|
||||||
|
- Eingehängt in `sanitizeContractBody` → läuft automatisch für alle
|
||||||
|
fünf Identifier-Felder bei Create/Update. ContractForm bekommt
|
||||||
|
`maxLength={100}` als UX-Schicht. Doku in
|
||||||
|
`docs/SECURITY-HARDENING.md` § Runde 86.
|
||||||
|
|
||||||
- [x] **🆕 Vertrag: Auftragsnummer bei Vertriebsplattform**
|
- [x] **🆕 Vertrag: Auftragsnummer bei Vertriebsplattform**
|
||||||
- Neues optionales Feld `Contract.orderNumberAtSalesPlatform`
|
- Neues optionales Feld `Contract.orderNumberAtSalesPlatform`
|
||||||
(`VARCHAR(191) NULL`), Migration
|
(`VARCHAR(191) NULL`), Migration
|
||||||
|
|||||||
@@ -956,11 +956,11 @@ export default function ContractForm() {
|
|||||||
options={availableTariffs.map((t) => ({ value: t.id, label: t.name }))}
|
options={availableTariffs.map((t) => ({ value: t.id, label: t.name }))}
|
||||||
disabled={!selectedProviderId}
|
disabled={!selectedProviderId}
|
||||||
/>
|
/>
|
||||||
<Input label="Kundennummer beim Anbieter" {...register('customerNumberAtProvider')} />
|
<Input label="Kundennummer beim Anbieter" maxLength={100} {...register('customerNumberAtProvider')} />
|
||||||
<Input label="Vertragsnummer beim Anbieter" {...register('contractNumberAtProvider')} />
|
<Input label="Vertragsnummer beim Anbieter" maxLength={100} {...register('contractNumberAtProvider')} />
|
||||||
<Input label="Auftragsnummer bei Vertriebsplattform" {...register('orderNumberAtSalesPlatform')} />
|
<Input label="Auftragsnummer bei Vertriebsplattform" maxLength={100} {...register('orderNumberAtSalesPlatform')} />
|
||||||
<Input label="Kundennummer bei Vertriebsplattform" {...register('customerNumberAtSalesPlatform')} />
|
<Input label="Kundennummer bei Vertriebsplattform" maxLength={100} {...register('customerNumberAtSalesPlatform')} />
|
||||||
<Input label="Vertragsnummer bei Vertriebsplattform" {...register('contractNumberAtSalesPlatform')} />
|
<Input label="Vertragsnummer bei Vertriebsplattform" maxLength={100} {...register('contractNumberAtSalesPlatform')} />
|
||||||
<Input label="Provision (€)" type="number" step="0.01" {...register('commission')} />
|
<Input label="Provision (€)" type="number" step="0.01" {...register('commission')} />
|
||||||
<Input label="Preis erste 12 Monate" {...register('priceFirst12Months')} placeholder="z.B. 29,99 €/Monat" />
|
<Input label="Preis erste 12 Monate" {...register('priceFirst12Months')} placeholder="z.B. 29,99 €/Monat" />
|
||||||
<Input label="Preis ab 13. Monat" {...register('priceFrom13Months')} placeholder="z.B. 39,99 €/Monat" />
|
<Input label="Preis ab 13. Monat" {...register('priceFrom13Months')} placeholder="z.B. 39,99 €/Monat" />
|
||||||
|
|||||||
Reference in New Issue
Block a user