Security-Hardening Runde 12: Information-Disclosure + Input-Validation
Pentest Runde 7 (Anschlussrunde):
MEDIUM – Interne Felder in Portal-Responses:
- sanitizeCustomerStrict strippt zusätzlich portalTokenInvalidatedAt,
portalLastLogin, portalPasswordMustChange, lastBirthdayGreetingYear,
privacyPolicyPath, businessRegistrationPath, commercialRegisterPath.
- Neue sanitizeContract/Strict + sanitizeContracts/Strict: entfernt
portalPasswordEncrypted immer (nur über /password-Endpoint mit Audit
abrufbar), für Portal-User zusätzlich commission/notes/nextReviewDate.
- getContract + getContracts wählen je nach isCustomerPortal die
passende Variante. Mitarbeiter sehen commission/notes weiterhin.
LOW – Integer-Truncation bei IDs:
parseInt('6abc') → 6 lief vorher durch. Neue Heuristik-Middleware
unter /api: jedes Pfad-Segment, das mit Ziffer beginnt aber nicht
aus reinen Ziffern besteht, wird mit 400 abgelehnt. Trifft alle
Sub-Router ohne dass jede Route einzeln angefasst werden muss.
INFO – Rate-Limit: Code-Stand limit=10 für Login, limit=5 für
Password-Reset (lokal verifiziert: 11. failed login = 429). Pentester
sah vermutlich noch älteren Build. Kein Code-Change.
Live-verifiziert:
- /customers/6abc → 400 "Ungültige ID im URL-Pfad"
- /customers/3 → 200, /contracts/1abc/history → 400, normale Pfade OK
- Portal-User /customers/3: keine portalLastLogin/portalPasswordMustChange/
portalTokenInvalidatedAt/etc. mehr in Response
- Portal-User /contracts/15: keine commission/notes/portalPasswordEncrypted/
nextReviewDate
- Admin /contracts/15: commission/notes/nextReviewDate sichtbar,
portalPasswordEncrypted weg
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -6,6 +6,7 @@ import * as contractHistoryService from '../services/contractHistory.service.js'
|
|||||||
import * as authorizationService from '../services/authorization.service.js';
|
import * as authorizationService from '../services/authorization.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 } from '../utils/sanitize.js';
|
||||||
import { canAccessContract } from '../utils/accessControl.js';
|
import { canAccessContract } from '../utils/accessControl.js';
|
||||||
import { maybeActivateOnDeliveryConfirmation } from '../services/contractStatusScheduler.service.js';
|
import { maybeActivateOnDeliveryConfirmation } from '../services/contractStatusScheduler.service.js';
|
||||||
|
|
||||||
@@ -46,9 +47,15 @@ export async function getContracts(req: AuthRequest, res: Response): Promise<voi
|
|||||||
page: page ? parseInt(page as string) : undefined,
|
page: page ? parseInt(page as string) : undefined,
|
||||||
limit: limit ? parseInt(limit as string) : undefined,
|
limit: limit ? parseInt(limit as string) : undefined,
|
||||||
});
|
});
|
||||||
|
// Portal-User bekommen die Strict-Variante (ohne commission/notes/
|
||||||
|
// nextReviewDate/portalPasswordEncrypted), Mitarbeiter die normale.
|
||||||
|
const isPortal = !!req.user?.isCustomerPortal;
|
||||||
|
const data = isPortal
|
||||||
|
? sanitizeContractsStrict(result.contracts as any[])
|
||||||
|
: sanitizeContracts(result.contracts as any[]);
|
||||||
res.json({
|
res.json({
|
||||||
success: true,
|
success: true,
|
||||||
data: result.contracts,
|
data,
|
||||||
pagination: result.pagination,
|
pagination: result.pagination,
|
||||||
} as ApiResponse);
|
} as ApiResponse);
|
||||||
} catch (error) {
|
} catch (error) {
|
||||||
@@ -89,7 +96,11 @@ export async function getContract(req: AuthRequest, res: Response): Promise<void
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
res.json({ success: true, data: contract } as ApiResponse);
|
const isPortal = !!req.user?.isCustomerPortal;
|
||||||
|
const data = isPortal
|
||||||
|
? sanitizeContractStrict(contract as any)
|
||||||
|
: sanitizeContract(contract as any);
|
||||||
|
res.json({ success: true, data } as ApiResponse);
|
||||||
} catch (error) {
|
} catch (error) {
|
||||||
res.status(500).json({
|
res.status(500).json({
|
||||||
success: false,
|
success: false,
|
||||||
|
|||||||
@@ -262,6 +262,25 @@ app.use('/api', (_req, res, next) => {
|
|||||||
next();
|
next();
|
||||||
});
|
});
|
||||||
|
|
||||||
|
// Numerische ID-Parameter strikt validieren. parseInt('6abc') liefert 6, was
|
||||||
|
// dazu führt, dass `/api/customers/6abc` als `/api/customers/6` interpretiert
|
||||||
|
// wurde – kein Auth-Bypass (Prisma fängt SQL-Injection), aber fehlende Input-
|
||||||
|
// Validierung. Pentest Runde 7 (2026-05-17), LOW.
|
||||||
|
//
|
||||||
|
// `app.param()` greift nicht auf in Sub-Router gemounteten Routes, deshalb
|
||||||
|
// machen wir es als Pfad-Heuristik: jedes Segment, das mit einer Ziffer
|
||||||
|
// beginnt aber nicht aus reinen Ziffern besteht (`6abc`, `12foo`), wird als
|
||||||
|
// Tippfehler/Manipulation behandelt.
|
||||||
|
app.use('/api', (req, res, next) => {
|
||||||
|
for (const seg of req.path.split('/')) {
|
||||||
|
if (seg.length > 0 && /^\d/.test(seg) && !/^\d+$/.test(seg)) {
|
||||||
|
res.status(400).json({ success: false, error: 'Ungültige ID im URL-Pfad' });
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
next();
|
||||||
|
});
|
||||||
|
|
||||||
// Öffentliche Routes (OHNE Authentifizierung)
|
// Öffentliche Routes (OHNE Authentifizierung)
|
||||||
app.use('/api/public/consent', consentPublicRoutes);
|
app.use('/api/public/consent', consentPublicRoutes);
|
||||||
|
|
||||||
|
|||||||
@@ -17,6 +17,37 @@ const SENSITIVE_CUSTOMER_FIELDS = [
|
|||||||
'consentHash',
|
'consentHash',
|
||||||
] as const;
|
] as const;
|
||||||
|
|
||||||
|
// Zusätzliche Felder die Portal-User nicht in ihrer Customer-Response sehen
|
||||||
|
// sollen – Interne Session-/Workflow-State, kein direkter Auth-Bypass, aber
|
||||||
|
// unnötige Informationsleckage über den DB-Aufbau.
|
||||||
|
// Pentest Runde 7 (2026-05-17), MEDIUM.
|
||||||
|
const PORTAL_HIDDEN_CUSTOMER_FIELDS = [
|
||||||
|
'portalTokenInvalidatedAt',
|
||||||
|
'portalLastLogin',
|
||||||
|
'portalPasswordMustChange',
|
||||||
|
'lastBirthdayGreetingYear',
|
||||||
|
// privacyPolicyPath etc. sind interne Datei-Pfade – Portal nutzt
|
||||||
|
// dedizierte PDF-Endpoints, nicht den Pfad direkt
|
||||||
|
'privacyPolicyPath',
|
||||||
|
'businessRegistrationPath',
|
||||||
|
'commercialRegisterPath',
|
||||||
|
] as const;
|
||||||
|
|
||||||
|
// Felder die im Contract NIE rausgehen dürfen (auch nicht an Mitarbeiter).
|
||||||
|
// portalPasswordEncrypted ist nur über den dedizierten /password-Endpoint
|
||||||
|
// (mit Audit-Log) abrufbar – im /contracts/:id selbst nutzlos.
|
||||||
|
const SENSITIVE_CONTRACT_FIELDS = [
|
||||||
|
'portalPasswordEncrypted',
|
||||||
|
] as const;
|
||||||
|
|
||||||
|
// Zusätzliche Felder die Portal-User nicht sehen sollen (interne CRM-Daten).
|
||||||
|
// Pentest Runde 7 (2026-05-17): commission + notes leakten an Portal-User.
|
||||||
|
const PORTAL_HIDDEN_CONTRACT_FIELDS = [
|
||||||
|
'commission',
|
||||||
|
'notes',
|
||||||
|
'nextReviewDate', // Snooze-Workflow ist internes Cockpit-Feature
|
||||||
|
] as const;
|
||||||
|
|
||||||
const SENSITIVE_USER_FIELDS = [
|
const SENSITIVE_USER_FIELDS = [
|
||||||
'password',
|
'password',
|
||||||
'passwordResetToken',
|
'passwordResetToken',
|
||||||
@@ -43,14 +74,18 @@ export function sanitizeCustomer<T extends Record<string, unknown>>(customer: T
|
|||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Entfernt portalPasswordEncrypted zusätzlich zu den anderen sensiblen Feldern.
|
* Entfernt portalPasswordEncrypted + portal-interne Workflow-Felder zusätzlich
|
||||||
* Für Kontexte in denen der Caller KEIN Admin ist (z.B. Portal-Kunde).
|
* zu den allgemein sensiblen Feldern. Für Kontexte in denen der Caller KEIN
|
||||||
|
* Admin ist (z.B. Portal-Kunde).
|
||||||
*/
|
*/
|
||||||
export function sanitizeCustomerStrict<T extends Record<string, unknown>>(customer: T | null): T | null {
|
export function sanitizeCustomerStrict<T extends Record<string, unknown>>(customer: T | null): T | null {
|
||||||
if (!customer) return customer;
|
if (!customer) return customer;
|
||||||
const copy = sanitizeCustomer(customer) as Record<string, unknown> | null;
|
const copy = sanitizeCustomer(customer) as Record<string, unknown> | null;
|
||||||
if (!copy) return null;
|
if (!copy) return null;
|
||||||
delete copy.portalPasswordEncrypted;
|
delete copy.portalPasswordEncrypted;
|
||||||
|
for (const field of PORTAL_HIDDEN_CUSTOMER_FIELDS) {
|
||||||
|
delete copy[field];
|
||||||
|
}
|
||||||
return copy as T;
|
return copy as T;
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -61,6 +96,49 @@ export function sanitizeCustomers<T extends Record<string, unknown>>(customers:
|
|||||||
return customers.map((c) => sanitizeCustomer(c)).filter((c): c is T => c !== null);
|
return customers.map((c) => sanitizeCustomer(c)).filter((c): c is T => c !== null);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Sanitize Contract-Objekt für alle Caller. Entfernt das verschlüsselte
|
||||||
|
* Provider-Passwort (nur über den dedizierten /password-Endpoint mit
|
||||||
|
* Audit-Log abrufbar) und sanitisiert das embedded customer.
|
||||||
|
*/
|
||||||
|
export function sanitizeContract<T extends Record<string, unknown>>(contract: T | null): T | null {
|
||||||
|
if (!contract) return contract;
|
||||||
|
const copy: Record<string, unknown> = { ...contract };
|
||||||
|
for (const field of SENSITIVE_CONTRACT_FIELDS) {
|
||||||
|
delete copy[field];
|
||||||
|
}
|
||||||
|
if (copy.customer && typeof copy.customer === 'object') {
|
||||||
|
copy.customer = sanitizeCustomer(copy.customer as Record<string, unknown>);
|
||||||
|
}
|
||||||
|
return copy as T;
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Sanitize Contract für Portal-User: zusätzlich werden interne CRM-Felder
|
||||||
|
* (Provision, Notizen, Snooze-Date) gestrippt und das embedded customer
|
||||||
|
* mit `sanitizeCustomerStrict` gefiltert. Pentest Runde 7 (2026-05-17).
|
||||||
|
*/
|
||||||
|
export function sanitizeContractStrict<T extends Record<string, unknown>>(contract: T | null): T | null {
|
||||||
|
if (!contract) return contract;
|
||||||
|
const copy = sanitizeContract(contract) as Record<string, unknown> | null;
|
||||||
|
if (!copy) return null;
|
||||||
|
for (const field of PORTAL_HIDDEN_CONTRACT_FIELDS) {
|
||||||
|
delete copy[field];
|
||||||
|
}
|
||||||
|
if (copy.customer && typeof copy.customer === 'object') {
|
||||||
|
copy.customer = sanitizeCustomerStrict(copy.customer as Record<string, unknown>);
|
||||||
|
}
|
||||||
|
return copy as T;
|
||||||
|
}
|
||||||
|
|
||||||
|
export function sanitizeContracts<T extends Record<string, unknown>>(contracts: T[]): T[] {
|
||||||
|
return contracts.map((c) => sanitizeContract(c)).filter((c): c is T => c !== null);
|
||||||
|
}
|
||||||
|
|
||||||
|
export function sanitizeContractsStrict<T extends Record<string, unknown>>(contracts: T[]): T[] {
|
||||||
|
return contracts.map((c) => sanitizeContractStrict(c)).filter((c): c is T => c !== null);
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Sanitize User-Objekt für API-Responses.
|
* Sanitize User-Objekt für API-Responses.
|
||||||
*/
|
*/
|
||||||
|
|||||||
@@ -97,6 +97,41 @@ isolierte Instanz (keine Multi-Tenancy im Code), Provisioning + Abrechnung
|
|||||||
|
|
||||||
## ✅ Erledigt
|
## ✅ Erledigt
|
||||||
|
|
||||||
|
- [x] **🚨 Pentest Runde 7 (Anschlussrunde) – Information-Disclosure + Input-Validation**
|
||||||
|
- **MEDIUM – Interne Felder in Portal-Responses**:
|
||||||
|
* `sanitizeCustomerStrict` strippt jetzt zusätzlich
|
||||||
|
`portalTokenInvalidatedAt`, `portalLastLogin`,
|
||||||
|
`portalPasswordMustChange`, `lastBirthdayGreetingYear`,
|
||||||
|
`privacyPolicyPath`, `businessRegistrationPath`,
|
||||||
|
`commercialRegisterPath`.
|
||||||
|
* Neue `sanitizeContract` / `sanitizeContractStrict` /
|
||||||
|
`sanitizeContracts(Strict)`: entfernt
|
||||||
|
`portalPasswordEncrypted` (immer; ist nur über den dedizierten
|
||||||
|
`/password`-Endpoint mit Audit-Log abrufbar) und für Portal-
|
||||||
|
User zusätzlich `commission`, `notes`, `nextReviewDate`.
|
||||||
|
* `getContract` + `getContracts` rufen jetzt die passende
|
||||||
|
Sanitize-Variante je nach `req.user.isCustomerPortal` auf;
|
||||||
|
Mitarbeiter sehen weiterhin commission/notes (Admin-Workflow),
|
||||||
|
nur `portalPasswordEncrypted` ist generell entfernt (Klartext
|
||||||
|
nur über dedicated Endpoint).
|
||||||
|
* Live-verifiziert: Portal sieht 0 Leaks, Admin sieht
|
||||||
|
commission/notes weiterhin.
|
||||||
|
- **LOW – Integer-Truncation bei IDs**:
|
||||||
|
`parseInt('6abc')` → `6` hat alle Endpoints durchgewunken.
|
||||||
|
Neuer middleware in `index.ts`: jedes URL-Pfad-Segment unter
|
||||||
|
`/api`, das mit Ziffer beginnt aber nicht aus reinen Ziffern
|
||||||
|
besteht, wird mit HTTP 400 abgelehnt. Heuristik trifft alle
|
||||||
|
`/resource/<id>(\D+)`-Patterns ohne dass jeder einzelne
|
||||||
|
Sub-Router angefasst werden muss.
|
||||||
|
* Live-verifiziert: `/customers/6abc` → 400 mit klarer Meldung,
|
||||||
|
`/customers/3` weiterhin 200, `/contracts/1abc/history`
|
||||||
|
→ 400, normaler Pfade `/audit-logs/customer/3` → 200.
|
||||||
|
- **INFO – Login-Rate-Limit „nach 6 nicht aktiv"**:
|
||||||
|
Code-Stand `limit: 10` für `loginRateLimiter`, lokal verifiziert:
|
||||||
|
11. Versuch = 429. Pentester sah vermutlich noch alten Build
|
||||||
|
oder eine andere Lokation (PW-Reset hat `limit: 5`). Kein
|
||||||
|
Code-Change.
|
||||||
|
|
||||||
- [x] **🛠 Rate-Limit-Sperren: Admin-UI zum Freigeben**
|
- [x] **🛠 Rate-Limit-Sperren: Admin-UI zum Freigeben**
|
||||||
- Bei einer Pentest-Runde hat der Tester sich selbst durch zu viele
|
- Bei einer Pentest-Runde hat der Tester sich selbst durch zu viele
|
||||||
Login-Versuche ausgesperrt → ohne Container-Restart kein Weg zurück.
|
Login-Versuche ausgesperrt → ohne Container-Restart kein Weg zurück.
|
||||||
|
|||||||
Reference in New Issue
Block a user