Pentest 2026-05-24 Pen-31-Befunde (2x MEDIUM)
31.1 Stored XSS in Vertragsfeldern: providerName, tariffName, priceFirst12Months, priceFrom13Months, priceAfter24Months nahmen rohe HTML-/Script-Payloads (<script>, <svg/onload>, <img onerror>, javascript:, HTML-Entities) an und lieferten sie 1:1 an Portal-User zurueck. Fix: rekursiver sanitizeContractBody()-Walker im contract.controller, strippt String-Werte ueber das bestehende stripHtml() (Tag-Strip + URI-Schema-Block + Entity-Decode). Verträge enthalten keine legitimen HTML-Felder, deshalb safe. Audit-Vergleich nutzt jetzt die sanitisierte Variante, sonst Audit ↔ DB-Drift. 31.2 IDOR auf GET /api/customers/:id/stressfrei-emails (+5 weitere): requireCustomerAccess short-circuitete auf customers:read. Portal- User haben aber genau diese Perm im JWT (für eigene Daten) – damit kam Portal-Kunde 1 an Adressen/Bank-Cards/Documents/Meters/ Stressfrei-Emails von Kunde 3. Fix im Middleware: erst isCustomerPortal-Check (eigene + vertretene IDs), DANN erst Perm-Check für Mitarbeiter. Mit einem Patch alle sechs requireCustomerAccess-Routes dicht. Defense-in-Depth: zusätzlicher canAccessCustomer-Call in stressfreiEmail.getEmailsByCustomer analog zum POST-Handler. Live-verifiziert auf dev: - Portal-User 1 → Customer 3: alle 6 Routes 403 - XSS-Payloads in 5 Contract-Feldern → DB enthält bereinigte Werte Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -6,10 +6,33 @@ import * as contractHistoryService from '../services/contractHistory.service.js'
|
||||
import * as authorizationService from '../services/authorization.service.js';
|
||||
import { ApiResponse, AuthRequest } from '../types/index.js';
|
||||
import { logChange } from '../services/audit.service.js';
|
||||
import { sanitizeContract, sanitizeContractStrict, sanitizeContracts, sanitizeContractsStrict } from '../utils/sanitize.js';
|
||||
import { sanitizeContract, sanitizeContractStrict, sanitizeContracts, sanitizeContractsStrict, stripHtml } from '../utils/sanitize.js';
|
||||
import { canAccessContract } from '../utils/accessControl.js';
|
||||
import { maybeActivateOnDeliveryConfirmation } from '../services/contractStatusScheduler.service.js';
|
||||
|
||||
/**
|
||||
* Walk-and-clean: strippt HTML/Script-/URI-Schemata in allen String-Werten
|
||||
* eines Body-Objekts (rekursiv über energyDetails, internetDetails etc.).
|
||||
* Pentest 2026-05-24 (MEDIUM, 31.1): providerName, tariffName und die
|
||||
* price*-Felder nahmen rohe HTML-Payloads an (`<script>`, `<svg onload>`)
|
||||
* und lieferten sie 1:1 an Portal-User zurück. Verträge enthalten KEINE
|
||||
* HTML-Felder (Richtige HTML-Texte liegen in AppSettings), deshalb ist
|
||||
* Strip safe.
|
||||
*/
|
||||
function sanitizeContractBody(body: unknown): unknown {
|
||||
if (body === null || body === undefined) return body;
|
||||
if (typeof body === 'string') return stripHtml(body);
|
||||
if (Array.isArray(body)) return body.map(sanitizeContractBody);
|
||||
if (typeof body === 'object') {
|
||||
const out: Record<string, unknown> = {};
|
||||
for (const [k, v] of Object.entries(body as Record<string, unknown>)) {
|
||||
out[k] = sanitizeContractBody(v);
|
||||
}
|
||||
return out;
|
||||
}
|
||||
return body;
|
||||
}
|
||||
|
||||
export async function getContracts(req: AuthRequest, res: Response): Promise<void> {
|
||||
try {
|
||||
const { customerId, type, status, search, page, limit, tree } = req.query;
|
||||
@@ -122,7 +145,8 @@ export async function createContract(req: AuthRequest, res: Response): Promise<v
|
||||
res.status(400).json({ success: false, error: 'Kunde (customerId) ist erforderlich' } as ApiResponse);
|
||||
return;
|
||||
}
|
||||
const contract = await contractService.createContract(req.body);
|
||||
const sanitizedBody = sanitizeContractBody(body);
|
||||
const contract = await contractService.createContract(sanitizedBody as any);
|
||||
await logChange({
|
||||
req, action: 'CREATE', resourceType: 'Contract',
|
||||
resourceId: contract.id.toString(),
|
||||
@@ -149,7 +173,9 @@ export async function updateContract(req: AuthRequest, res: Response): Promise<v
|
||||
include: { energyDetails: true, internetDetails: true, mobileDetails: true, tvDetails: true, carInsuranceDetails: true },
|
||||
});
|
||||
|
||||
const contract = await contractService.updateContract(contractId, req.body);
|
||||
// HTML/JS-Strip auf allen String-Werten (Pentest 2026-05-24, 31.1)
|
||||
const sanitizedBody = sanitizeContractBody(req.body);
|
||||
const contract = await contractService.updateContract(contractId, sanitizedBody as any);
|
||||
|
||||
// Geänderte Felder ermitteln
|
||||
const changes: Record<string, { von: unknown; nach: unknown }> = {};
|
||||
@@ -168,8 +194,10 @@ export async function updateContract(req: AuthRequest, res: Response): Promise<v
|
||||
instantBonus: 'Sofort-Bonus', newCustomerBonus: 'Neukunden-Bonus',
|
||||
};
|
||||
|
||||
// Hauptfelder vergleichen
|
||||
const body = req.body;
|
||||
// Hauptfelder vergleichen – gegen die SANITISIERTE Version, damit
|
||||
// das Audit-Log die echten DB-Werte widerspiegelt, nicht den
|
||||
// rohen Request-Body mit ggf. gestrippter HTML.
|
||||
const body = sanitizedBody as any;
|
||||
if (before) {
|
||||
for (const [key, newVal] of Object.entries(body)) {
|
||||
if (['energyDetails', 'internetDetails', 'mobileDetails', 'tvDetails', 'carInsuranceDetails', 'password'].includes(key)) continue;
|
||||
|
||||
@@ -2,11 +2,17 @@ import { Request, Response } from 'express';
|
||||
import * as stressfreiEmailService from '../services/stressfreiEmail.service.js';
|
||||
import { logChange } from '../services/audit.service.js';
|
||||
import { ApiResponse, AuthRequest } from '../types/index.js';
|
||||
import { canAccessStressfreiEmail } from '../utils/accessControl.js';
|
||||
import { canAccessCustomer, canAccessStressfreiEmail } from '../utils/accessControl.js';
|
||||
|
||||
export async function getEmailsByCustomer(req: Request, res: Response): Promise<void> {
|
||||
export async function getEmailsByCustomer(req: AuthRequest, res: Response): Promise<void> {
|
||||
try {
|
||||
const customerId = parseInt(req.params.customerId);
|
||||
// requireCustomerAccess in der Route greift nicht ausreichend:
|
||||
// Portal-User haben `customers:read` (für eigene Daten) und werden
|
||||
// dort short-circuited, ohne Owner-Vergleich. Pentest 2026-05-24
|
||||
// (MEDIUM 31.2) – IDOR auf fremde IMAP-Konten. Hier daher der
|
||||
// explizite Per-Customer-Check analog zum POST-Handler.
|
||||
if (!(await canAccessCustomer(req, res, customerId))) return;
|
||||
const includeInactive = req.query.includeInactive === 'true';
|
||||
const emails = await stressfreiEmailService.getEmailsByCustomerId(customerId, includeInactive);
|
||||
res.json({ success: true, data: emails } as ApiResponse);
|
||||
|
||||
@@ -158,9 +158,34 @@ export function requireCustomerAccess(
|
||||
return;
|
||||
}
|
||||
|
||||
// WICHTIG: erst die isCustomerPortal-Prüfung, DANN erst die Perm-Prüfung.
|
||||
// Portal-User bekommen `customers:read` im JWT (für eigene Daten); ohne
|
||||
// den Portal-Check vorne weg short-circuited die alte Logik auf der
|
||||
// Perm und ließ Portal-User auf fremde customerId zugreifen.
|
||||
// Pentest 2026-05-24 (MEDIUM 31.2 IDOR auf /api/customers/:id/
|
||||
// stressfrei-emails). Auch andere Routes mit dem gleichen Middleware-
|
||||
// Pattern wären betroffen gewesen.
|
||||
const userPermissions = req.user.permissions || [];
|
||||
const isPortal = !!(req.user as any).isCustomerPortal;
|
||||
const customerId = parseInt(req.params.customerId || req.params.id);
|
||||
|
||||
// Admins and employees can access all customers
|
||||
if (isPortal) {
|
||||
const allowedIds = [
|
||||
req.user.customerId,
|
||||
...((req.user as any).representedCustomerIds || []),
|
||||
].filter(Boolean);
|
||||
if (allowedIds.includes(customerId)) {
|
||||
next();
|
||||
return;
|
||||
}
|
||||
res.status(403).json({
|
||||
success: false,
|
||||
error: 'Kein Zugriff auf diese Kundendaten',
|
||||
});
|
||||
return;
|
||||
}
|
||||
|
||||
// Mitarbeiter/Admin: customers:read oder customers:update reicht
|
||||
if (
|
||||
userPermissions.includes('customers:read') ||
|
||||
userPermissions.includes('customers:update')
|
||||
@@ -169,18 +194,6 @@ export function requireCustomerAccess(
|
||||
return;
|
||||
}
|
||||
|
||||
// Customers can only access their own data + represented customers
|
||||
const customerId = parseInt(req.params.customerId || req.params.id);
|
||||
const allowedIds = [
|
||||
req.user.customerId,
|
||||
...((req.user as any).representedCustomerIds || []),
|
||||
].filter(Boolean);
|
||||
|
||||
if (allowedIds.includes(customerId)) {
|
||||
next();
|
||||
return;
|
||||
}
|
||||
|
||||
res.status(403).json({
|
||||
success: false,
|
||||
error: 'Kein Zugriff auf diese Kundendaten',
|
||||
|
||||
@@ -120,6 +120,42 @@ isolierte Instanz (keine Multi-Tenancy im Code), Provisioning + Abrechnung
|
||||
- **Live-verifiziert**: 4867 Datensätze + 1 Datei in 13.2s
|
||||
wiederhergestellt, Log-Modal zeigt den vollständigen Verlauf.
|
||||
|
||||
- [x] **🛡️ Pentest 2026-05-24 Pen-31-Befunde (2× MEDIUM)**
|
||||
- **31.1 Stored XSS in Vertragsfeldern**: `providerName`, `tariffName`,
|
||||
`priceFirst12Months`, `priceFrom13Months`, `priceAfter24Months`
|
||||
nahmen rohe HTML/Script-Payloads an und lieferten sie 1:1 zurück.
|
||||
Fix: rekursiver `sanitizeContractBody()` (Walk-and-Strip) im
|
||||
contract.controller wird auf `req.body` von POST + PUT
|
||||
angewandt. Nutzt das bestehende `stripHtml()` aus utils/sanitize,
|
||||
inkl. URI-Schema-Block + Entity-Decode. Verträge enthalten keine
|
||||
legitimen HTML-Felder (Editor-HTML lebt in AppSettings), daher
|
||||
Strip ohne Risiko. Audit-Vergleich nutzt jetzt die sanitisierte
|
||||
Version, sonst Audit ↔ DB-Drift.
|
||||
- **31.2 IDOR auf `GET /customers/:id/stressfrei-emails`** (und 4
|
||||
weiteren Routes mit `requireCustomerAccess`): das Middleware
|
||||
short-circuitete auf `customers:read` – aber Portal-User haben
|
||||
diese Perm im JWT (für eigene Daten). Damit kam Portal-Kunde 1
|
||||
an IMAP-Konten/Adressen/Bank-Cards/Documents/Meters von
|
||||
Kunde 3. Fix in `middleware/auth.ts:requireCustomerAccess`:
|
||||
erst `isCustomerPortal`-Check (eigene + vertretene IDs), DANN
|
||||
erst Perm-Check für Mitarbeiter. Damit sind alle 6 Routes
|
||||
mit einem Middleware-Patch dicht. Defense-in-Depth: in
|
||||
`stressfreiEmail.controller.getEmailsByCustomer` zusätzlich
|
||||
`canAccessCustomer`-Call analog zum POST-Handler.
|
||||
- **Infos** (keine Code-Änderung):
|
||||
- `type:"STROM"` ist deprecated – richtige Enum ist `ELECTRICITY`.
|
||||
- HSTS auf Staging fehlt: HSTS macht der nginx-Reverse-Proxy,
|
||||
Backend setzt's bewusst nicht (Doppel-Header-Vermeidung).
|
||||
Auf Staging muss der Proxy-Op das HSTS-Header-Add aktivieren.
|
||||
- Portal-Login-Rate-Limit 5 vs 10: Env-Drift, identische Codebase.
|
||||
- **Live-verifiziert** auf dev:
|
||||
- Portal-User 1 vs Customer 3: alle 6 Routes 403
|
||||
(`/customers/3`, `.../addresses`, `.../bank-cards`,
|
||||
`.../documents`, `.../meters`, `.../stressfrei-emails`).
|
||||
- XSS-Payloads `<script>`, `<svg/onload>`, `<img onerror>`,
|
||||
`javascript:`, `<script>` in 5 Vertragsfeldern →
|
||||
DB-Werte bereinigt (`EvilProvider`, `blocked:alert(4) 35€` etc.).
|
||||
|
||||
- [x] **🆕 Vertragsansicht: Kunden-Schnellansicht-Modal + Cent/Euro-Doppel-Input**
|
||||
- **Info-Icon neben Kundennamen** öffnet ein Modal mit den
|
||||
wichtigsten Kundendaten (Firma, Name, Geburtsdatum/-ort,
|
||||
|
||||
Reference in New Issue
Block a user