From aa0900410bdcaac8767666359acb8384fdfffc37 Mon Sep 17 00:00:00 2001
From: duffyduck <info@hacker-net.de>
Date: Sun, 24 May 2026 15:38:16 +0200
Subject: [PATCH] Pentest 2026-05-24 Pen-31-Befunde (2x MEDIUM)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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>
---
 .../src/controllers/contract.controller.ts    | 38 +++++++++++++++---
 .../controllers/stressfreiEmail.controller.ts | 10 ++++-
 backend/src/middleware/auth.ts                | 39 ++++++++++++-------
 docs/todo.md                                  | 36 +++++++++++++++++
 4 files changed, 103 insertions(+), 20 deletions(-)

diff --git a/backend/src/controllers/contract.controller.ts b/backend/src/controllers/contract.controller.ts
index ea2a0151..4949a2bb 100644
--- a/backend/src/controllers/contract.controller.ts
+++ b/backend/src/controllers/contract.controller.ts
@@ -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;
diff --git a/backend/src/controllers/stressfreiEmail.controller.ts b/backend/src/controllers/stressfreiEmail.controller.ts
index a63d157f..cb6faa31 100644
--- a/backend/src/controllers/stressfreiEmail.controller.ts
+++ b/backend/src/controllers/stressfreiEmail.controller.ts
@@ -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);
diff --git a/backend/src/middleware/auth.ts b/backend/src/middleware/auth.ts
index 8b822176..2478b465 100644
--- a/backend/src/middleware/auth.ts
+++ b/backend/src/middleware/auth.ts
@@ -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',
diff --git a/docs/todo.md b/docs/todo.md
index 42fa694a..cba16e1c 100644
--- a/docs/todo.md
+++ b/docs/todo.md
@@ -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:`, `&#60;script&#62;` 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,