From 246999be01fee654437b71c887698dc8c61ae2f1 Mon Sep 17 00:00:00 2001 From: duffyduck Date: Thu, 18 Jun 2026 13:41:16 +0200 Subject: [PATCH] =?UTF-8?q?Pentest=2071.1-71.4:=20H=C3=A4rtung=20der=20Zus?= =?UTF-8?q?atz-Weiterleitungen?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 71.1 MEDIUM: BLOCKED_TLDS-Set in assertValidForwardingEmail – reservierte/private TLDs (local, internal, corp, lan, home, private, invalid, test, localhost, example, intranet, localdomain, arpa) werden abgelehnt. Schließt Plesk-DNS-Probing ins interne Netz. 71.2 LOW: canonicalEmailKey-Helper normalisiert Mail-Adressen für den Dedup (Plus-Tag wegstrippen, lowercase). billing+x@y und billing@y haben jetzt denselben Schlüssel – auch gegen Kunden- Stamm-Mail und gegen config.defaultForwardEmail im sync-Pfad. 71.3 INFO: Neuer requireIdParam-Helper im Controller liefert 400 statt 500 bei nicht-numerischen Route-IDs. Alle acht parseInt- Stellen umgestellt (auch über die gemeldete eine hinaus). 71.4 INFO: setAdditionalForwards rollt den DB-Stand zurück, wenn syncForwardingForEmail mit dem Provider scheitert. Vorheriger Wert wird vorm Update gemerkt und im Fehlerfall wieder eingespielt – DB und Plesk laufen nicht mehr auseinander. Smoke-Tests: 11 reservierte TLDs abgelehnt, 4 echte TLDs (de, com, co.uk, museum) durchgewinkt, Plus-Tag-Strip mit Multi-Plus+Casing. --- .../controllers/stressfreiEmail.controller.ts | 37 +++++-- .../src/services/stressfreiEmail.service.ts | 96 +++++++++++++++++-- docs/todo.md | 23 +++++ 3 files changed, 141 insertions(+), 15 deletions(-) diff --git a/backend/src/controllers/stressfreiEmail.controller.ts b/backend/src/controllers/stressfreiEmail.controller.ts index 1b4c8c32..5817bf24 100644 --- a/backend/src/controllers/stressfreiEmail.controller.ts +++ b/backend/src/controllers/stressfreiEmail.controller.ts @@ -5,9 +5,23 @@ import { ApiResponse, AuthRequest } from '../types/index.js'; import { canAccessCustomer, canAccessStressfreiEmail } from '../utils/accessControl.js'; import { ApiError } from '../utils/apiError.js'; +// Pentest 71.3 (INFO): `parseInt(...)` ohne NaN-Check gab bei +// `/stressfrei-emails/abc/...` einen generischen 500 zurück. Nicht +// kritisch, aber irreführend und log-spammend. +function requireIdParam(req: AuthRequest, res: Response, paramName: string): number | null { + const raw = req.params[paramName]; + const parsed = Number.parseInt(raw, 10); + if (!Number.isInteger(parsed) || parsed < 1) { + res.status(400).json({ success: false, error: `Ungültige ID: ${raw}` } as ApiResponse); + return null; + } + return parsed; +} + export async function getEmailsByCustomer(req: AuthRequest, res: Response): Promise { try { - const customerId = parseInt(req.params.customerId); + const customerId = requireIdParam(req, res, 'customerId'); + if (customerId === null) return; // 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 @@ -27,7 +41,8 @@ export async function getEmailsByCustomer(req: AuthRequest, res: Response): Prom export async function getEmail(req: AuthRequest, res: Response): Promise { try { - const emailId = parseInt(req.params.id); + const emailId = requireIdParam(req, res, 'id'); + if (emailId === null) return; if (!(await canAccessStressfreiEmail(req, res, emailId))) return; const email = await stressfreiEmailService.getEmailById(emailId); @@ -55,7 +70,8 @@ export async function getEmail(req: AuthRequest, res: Response): Promise { export async function createEmail(req: Request, res: Response): Promise { try { - const customerId = parseInt(req.params.customerId); + const customerId = requireIdParam(req, res, 'customerId'); + if (customerId === null) return; const email = await stressfreiEmailService.createEmail({ ...req.body, customerId, @@ -77,7 +93,8 @@ export async function createEmail(req: Request, res: Response): Promise { export async function updateEmail(req: AuthRequest, res: Response): Promise { try { - const emailId = parseInt(req.params.id); + const emailId = requireIdParam(req, res, 'id'); + if (emailId === null) return; if (!(await canAccessStressfreiEmail(req, res, emailId))) return; const email = await stressfreiEmailService.updateEmail(emailId, req.body); await logChange({ @@ -96,7 +113,8 @@ export async function updateEmail(req: AuthRequest, res: Response): Promise { try { - const emailId = parseInt(req.params.id); + const emailId = requireIdParam(req, res, 'id'); + if (emailId === null) return; if (!(await canAccessStressfreiEmail(req, res, emailId))) return; await stressfreiEmailService.deleteEmail(emailId); await logChange({ @@ -115,7 +133,8 @@ export async function deleteEmail(req: AuthRequest, res: Response): Promise { try { - const emailId = parseInt(req.params.id); + const emailId = requireIdParam(req, res, 'id'); + if (emailId === null) return; if (!(await canAccessStressfreiEmail(req, res, emailId))) return; const result = await stressfreiEmailService.syncForwardingForEmail(emailId); @@ -159,7 +178,8 @@ export async function syncForwarding(req: AuthRequest, res: Response): Promise { try { - const emailId = parseInt(req.params.id); + const emailId = requireIdParam(req, res, 'id'); + if (emailId === null) return; if (!(await canAccessStressfreiEmail(req, res, emailId))) return; const body = req.body ?? {}; @@ -202,7 +222,8 @@ export async function updateAdditionalForwards(req: AuthRequest, res: Response): export async function resetPassword(req: AuthRequest, res: Response): Promise { try { - const emailId = parseInt(req.params.id); + const emailId = requireIdParam(req, res, 'id'); + if (emailId === null) return; if (!(await canAccessStressfreiEmail(req, res, emailId))) return; const result = await stressfreiEmailService.resetMailboxPassword(emailId); if (!result.success) { diff --git a/backend/src/services/stressfreiEmail.service.ts b/backend/src/services/stressfreiEmail.service.ts index 48bb8fa6..edd8ac6f 100644 --- a/backend/src/services/stressfreiEmail.service.ts +++ b/backend/src/services/stressfreiEmail.service.ts @@ -17,6 +17,16 @@ import { ApiError } from '../utils/apiError.js'; // Komma). Wirklich validiert wird vom Provider beim Sync. const FORWARD_EMAIL_REGEX = /^[A-Za-z0-9._%+\-]+@[A-Za-z0-9.\-]+\.[A-Za-z]{2,}$/; +// Pentest 71.1 (MEDIUM, 2026-06-08): RFC-reservierte und private/ +// On-Prem-TLDs. Eine Weiterleitung an `attacker@plesk.internal` würde +// am Provider DNS-Lookups gegen internes Netz auslösen oder bei mDNS- +// Setup an einen lokalen Mailserver gehen. Wir blocken sie hart. +const BLOCKED_TLDS = new Set([ + 'local', 'internal', 'corp', 'lan', 'home', 'private', + 'invalid', 'test', 'localhost', 'example', + 'intranet', 'localdomain', 'arpa', +]); + export function parseAdditionalForwards(raw: string | null | undefined): string[] { if (!raw) return []; try { @@ -33,6 +43,23 @@ export function serializeAdditionalForwards(list: string[]): string | null { return cleaned.length === 0 ? null : JSON.stringify(cleaned); } +/** + * Liefert eine normalisierte Form der Adresse für den Dedup-Vergleich: + * lowercase + Plus-Tag aus dem Local-Part rausgestrippt. + * `billing+pentest@test.de` und `billing@test.de` haben so denselben + * Schlüssel und treffen sich beim Dedup. Pentest 71.2. + */ +export function canonicalEmailKey(email: string): string { + const trimmed = email.trim().toLowerCase(); + const at = trimmed.lastIndexOf('@'); + if (at < 1) return trimmed; + const localPart = trimmed.slice(0, at); + const domain = trimmed.slice(at + 1); + const plus = localPart.indexOf('+'); + const cleanedLocal = plus === -1 ? localPart : localPart.slice(0, plus); + return `${cleanedLocal}@${domain}`; +} + export function assertValidForwardingEmail(value: unknown): string { if (typeof value !== 'string') { throw new ApiError(400, 'Ungültige Weiterleitungs-E-Mail-Adresse'); @@ -44,6 +71,13 @@ export function assertValidForwardingEmail(value: unknown): string { if (!FORWARD_EMAIL_REGEX.test(trimmed)) { throw new ApiError(400, `Ungültiges E-Mail-Format: ${trimmed}`); } + // 71.1: TLD aus dem Domain-Part rausziehen und gegen die Blocklist + // halten. Domain liegt nach dem letzten @, TLD nach dem letzten Punkt. + const domain = trimmed.slice(trimmed.lastIndexOf('@') + 1).toLowerCase(); + const tld = domain.slice(domain.lastIndexOf('.') + 1); + if (BLOCKED_TLDS.has(tld)) { + throw new ApiError(400, `Top-Level-Domain "${tld}" ist nicht erlaubt (reservierte/private TLD).`); + } return trimmed.toLowerCase(); } @@ -202,30 +236,73 @@ export async function deleteEmail(id: number) { * Komplette Liste zusätzlicher Weiterleitungs-E-Mails ersetzen und * direkt mit dem Provider synchronisieren. Aufrufer hat eine canonical * Liste – das Sub-Modal arbeitet auf Snapshot-Basis. + * + * Pentest 71.2: Dedup über `canonicalEmailKey` (Plus-Tags strippen), + * damit `billing+tag@x.de` und `billing@x.de` als gleiches Ziel + * erkannt werden – auch im Vergleich zur Stamm-E-Mail des Kunden. + * + * Pentest 71.4: DB-Update wird bei Provider-Sync-Fehler zurückgerollt, + * damit Plesk und DB nicht auseinanderlaufen. */ export async function setAdditionalForwards( id: number, emails: string[], ): Promise<{ success: boolean; forwardTargets?: string[]; error?: string }> { - // Input normalisieren + Duplikate raus (case-insensitive). + // Kunden-Stamm-Mail holen für Dedup gegen das (immer mit-gesetzte) Default-Ziel. + const meta = await prisma.stressfreiEmail.findUnique({ + where: { id }, + select: { + additionalForwardingEmails: true, + customer: { select: { email: true } }, + }, + }); + if (!meta) { + throw new ApiError(404, 'StressfreiEmail nicht gefunden'); + } + const previousRaw = meta.additionalForwardingEmails; + const customerEmailKey = meta.customer?.email + ? canonicalEmailKey(meta.customer.email) + : null; + + // Input normalisieren + Duplikate raus. const seen = new Set(); + if (customerEmailKey) seen.add(customerEmailKey); const cleaned: string[] = []; for (const raw of emails) { const ok = assertValidForwardingEmail(raw); - if (!seen.has(ok)) { - seen.add(ok); + const key = canonicalEmailKey(ok); + if (!seen.has(key)) { + seen.add(key); cleaned.push(ok); } } + const nextRaw = serializeAdditionalForwards(cleaned); await prisma.stressfreiEmail.update({ where: { id }, - data: { additionalForwardingEmails: serializeAdditionalForwards(cleaned) }, + data: { additionalForwardingEmails: nextRaw }, }); // Provider unmittelbar nachziehen, sonst läuft das Plesk-Mail-Konto // mit der alten Liste weiter. - return syncForwardingForEmail(id); + const syncResult = await syncForwardingForEmail(id); + + // 71.4: Rollback wenn Plesk den Sync abgelehnt hat. DB darf nicht + // den optimistischen Stand zeigen, wenn der Provider noch auf dem + // alten Stand ist. + if (!syncResult.success && previousRaw !== nextRaw) { + await prisma.stressfreiEmail.update({ + where: { id }, + data: { additionalForwardingEmails: previousRaw }, + }).catch((rollbackErr) => { + console.error( + '[setAdditionalForwards] Rollback nach Provider-Fail fehlgeschlagen:', + rollbackErr, + ); + }); + } + + return syncResult; } // Mailbox nachträglich aktivieren (für existierende E-Mail-Weiterleitung) @@ -401,9 +478,14 @@ export async function syncForwardingForEmail( } // Zusätzliche Weiterleitungsziele (vom User im Modal gepflegt). Duplikate // gegen die Stamm-Mail oder den Default werden hier weggefiltert, damit - // Plesk nicht mit Wiederholungen die Liste aufbläht. + // Plesk nicht mit Wiederholungen die Liste aufbläht. Pentest 71.2: + // Vergleich über `canonicalEmailKey`, damit Plus-Tags nicht doppelt + // zustellen. + const seenKeys = new Set(forwardTargets.map(canonicalEmailKey)); for (const extra of parseAdditionalForwards(stressfreiEmail.additionalForwardingEmails)) { - if (!forwardTargets.some((t) => t.toLowerCase() === extra.toLowerCase())) { + const key = canonicalEmailKey(extra); + if (!seenKeys.has(key)) { + seenKeys.add(key); forwardTargets.push(extra); } } diff --git a/docs/todo.md b/docs/todo.md index a6b558f6..49c75dd0 100644 --- a/docs/todo.md +++ b/docs/todo.md @@ -97,6 +97,29 @@ isolierte Instanz (keine Multi-Tenancy im Code), Provisioning + Abrechnung ## ✅ Erledigt +- [x] **🔒 Pentest 71.1–71.4: Härtung der Zusatz-Weiterleitungen** + - **71.1 MEDIUM:** Reservierte/private TLDs (`local`, `internal`, + `corp`, `lan`, `home`, `private`, `invalid`, `test`, `localhost`, + `example`, `intranet`, `localdomain`, `arpa`) werden in + `assertValidForwardingEmail` jetzt hart abgelehnt. Verhindert + Plesk-DNS-Probing ins interne Netz bei On-Prem-Setups. + - **71.2 LOW:** Neuer Helper `canonicalEmailKey` normalisiert Mail- + Adressen für den Dedup-Vergleich (Plus-Tag wegstrippen, + lowercase). `billing+pentest@x.de` und `billing@x.de` werden als + dasselbe Ziel erkannt – auch im Vergleich zur Kunden-Stamm-Mail + und im sync-Pfad gegen `config.defaultForwardEmail`. + - **71.3 INFO:** Neuer `requireIdParam(req, res, paramName)`-Helper + fängt nicht-numerische Route-Parameter und liefert 400 statt 500. + Alle acht parseInt-Stellen in `stressfreiEmail.controller.ts` + umgestellt (auch über das gemeldete Finding hinaus). + - **71.4 INFO:** `setAdditionalForwards` rollt den DB-Stand bei + Provider-Sync-Fehler zurück, damit DB und Plesk nicht + auseinanderlaufen. Vorheriger `additionalForwardingEmails`-Wert + wird vor dem Update gemerkt und bei Fail wieder eingespielt. + - Smoke-Tests bestätigen: 11 reservierte TLDs abgelehnt, 4 echte + TLDs (`de`, `com`, `co.uk`, `museum`) durchgewinkt, Plus-Tag- + Strip funktioniert (auch mit Multi-Plus + Casing). + - [x] **🆕 Stressfrei-Adressen: Zusatz-Weiterleitungen auch beim Anlegen** - Im „Adresse hinzufügen"-Modal erscheint der „Weitere Weiterleitungen"-Button jetzt auch, sobald „Beim E-Mail-Provider