diff --git a/backend/src/controllers/cancellation-period.controller.ts b/backend/src/controllers/cancellation-period.controller.ts index a25c15f5..93491937 100644 --- a/backend/src/controllers/cancellation-period.controller.ts +++ b/backend/src/controllers/cancellation-period.controller.ts @@ -2,6 +2,7 @@ import { Request, Response } from 'express'; import * as cancellationPeriodService from '../services/cancellation-period.service.js'; import { logChange } from '../services/audit.service.js'; import { ApiResponse } from '../types/index.js'; +import { pickCancellationPeriodUpdate } from '../utils/sanitize.js'; export async function getCancellationPeriods(req: Request, res: Response): Promise { try { @@ -54,7 +55,8 @@ export async function createCancellationPeriod(req: Request, res: Response): Pro export async function updateCancellationPeriod(req: Request, res: Response): Promise { try { - const period = await cancellationPeriodService.updateCancellationPeriod(parseInt(req.params.id), req.body); + // Pentest R110: Mass-Assignment-Whitelist. + const period = await cancellationPeriodService.updateCancellationPeriod(parseInt(req.params.id), pickCancellationPeriodUpdate(req.body)); await logChange({ req, action: 'UPDATE', resourceType: 'CancellationPeriod', resourceId: period.id.toString(), diff --git a/backend/src/controllers/contract-duration.controller.ts b/backend/src/controllers/contract-duration.controller.ts index db428300..567fb19b 100644 --- a/backend/src/controllers/contract-duration.controller.ts +++ b/backend/src/controllers/contract-duration.controller.ts @@ -2,6 +2,7 @@ import { Request, Response } from 'express'; import * as contractDurationService from '../services/contract-duration.service.js'; import { logChange } from '../services/audit.service.js'; import { ApiResponse } from '../types/index.js'; +import { pickContractDurationUpdate } from '../utils/sanitize.js'; export async function getContractDurations(req: Request, res: Response): Promise { try { @@ -54,7 +55,8 @@ export async function createContractDuration(req: Request, res: Response): Promi export async function updateContractDuration(req: Request, res: Response): Promise { try { - const duration = await contractDurationService.updateContractDuration(parseInt(req.params.id), req.body); + // Pentest R110: Mass-Assignment-Whitelist. + const duration = await contractDurationService.updateContractDuration(parseInt(req.params.id), pickContractDurationUpdate(req.body)); await logChange({ req, action: 'UPDATE', resourceType: 'ContractDuration', resourceId: duration.id.toString(), diff --git a/backend/src/controllers/contractCategory.controller.ts b/backend/src/controllers/contractCategory.controller.ts index 0c407a97..ba664d3b 100644 --- a/backend/src/controllers/contractCategory.controller.ts +++ b/backend/src/controllers/contractCategory.controller.ts @@ -2,6 +2,7 @@ import { Request, Response } from 'express'; import * as contractCategoryService from '../services/contractCategory.service.js'; import { logChange } from '../services/audit.service.js'; import { ApiResponse } from '../types/index.js'; +import { pickContractCategoryUpdate } from '../utils/sanitize.js'; export async function getContractCategories(req: Request, res: Response): Promise { try { @@ -54,7 +55,8 @@ export async function createContractCategory(req: Request, res: Response): Promi export async function updateContractCategory(req: Request, res: Response): Promise { try { - const category = await contractCategoryService.updateContractCategory(parseInt(req.params.id), req.body); + // Pentest R110: Mass-Assignment-Whitelist. + const category = await contractCategoryService.updateContractCategory(parseInt(req.params.id), pickContractCategoryUpdate(req.body)); await logChange({ req, action: 'UPDATE', resourceType: 'ContractCategory', resourceId: category.id.toString(), diff --git a/backend/src/controllers/emailProvider.controller.ts b/backend/src/controllers/emailProvider.controller.ts index 5d79a450..9ec95e65 100644 --- a/backend/src/controllers/emailProvider.controller.ts +++ b/backend/src/controllers/emailProvider.controller.ts @@ -10,6 +10,7 @@ import { decrypt } from '../utils/encryption.js'; import { assertAllowedHost, safeResolveHost } from '../utils/ssrfGuard.js'; import { emit as emitSecurityEvent, contextFromRequest } from '../services/securityMonitor.service.js'; import { PrismaClient } from '@prisma/client'; +import { pickEmailProviderUpdate } from '../utils/sanitize.js'; const prisma = new PrismaClient(); @@ -69,7 +70,9 @@ export async function createProviderConfig(req: Request, res: Response): Promise export async function updateProviderConfig(req: Request, res: Response): Promise { try { const id = parseInt(req.params.id); - const config = await emailProviderService.updateProviderConfig(id, req.body); + // Pentest R110: Mass-Assignment-Whitelist. Kein stripHtml, weil das + // Payload Passwörter/API-Keys enthalten kann. + const config = await emailProviderService.updateProviderConfig(id, pickEmailProviderUpdate(req.body) as any); await logChange({ req, action: 'UPDATE', resourceType: 'EmailProviderConfig', resourceId: id.toString(), diff --git a/backend/src/controllers/platform.controller.ts b/backend/src/controllers/platform.controller.ts index 870b2457..410c6d07 100644 --- a/backend/src/controllers/platform.controller.ts +++ b/backend/src/controllers/platform.controller.ts @@ -2,6 +2,7 @@ import { Request, Response } from 'express'; import * as platformService from '../services/platform.service.js'; import { logChange } from '../services/audit.service.js'; import { ApiResponse } from '../types/index.js'; +import { pickPlatformUpdate } from '../utils/sanitize.js'; export async function getPlatforms(req: Request, res: Response): Promise { try { @@ -54,7 +55,8 @@ export async function createPlatform(req: Request, res: Response): Promise export async function updatePlatform(req: Request, res: Response): Promise { try { - const platform = await platformService.updatePlatform(parseInt(req.params.id), req.body); + // Pentest R110: Mass-Assignment-Whitelist (nur name/contactInfo/isActive). + const platform = await platformService.updatePlatform(parseInt(req.params.id), pickPlatformUpdate(req.body)); await logChange({ req, action: 'UPDATE', resourceType: 'Platform', resourceId: platform.id.toString(), diff --git a/backend/src/controllers/stressfreiEmail.controller.ts b/backend/src/controllers/stressfreiEmail.controller.ts index f7e949f1..f41f260f 100644 --- a/backend/src/controllers/stressfreiEmail.controller.ts +++ b/backend/src/controllers/stressfreiEmail.controller.ts @@ -4,6 +4,7 @@ import { logChange } from '../services/audit.service.js'; import { ApiResponse, AuthRequest } from '../types/index.js'; import { canAccessCustomer, canAccessStressfreiEmail } from '../utils/accessControl.js'; import { ApiError } from '../utils/apiError.js'; +import { pickStressfreiEmailUpdate } from '../utils/sanitize.js'; // Pentest 71.3 (INFO): `parseInt(...)` ohne NaN-Check gab bei // `/stressfrei-emails/abc/...` einen generischen 500 zurück. @@ -105,7 +106,12 @@ export async function updateEmail(req: AuthRequest, res: Response): Promise { try { @@ -56,7 +57,8 @@ export async function createTariff(req: Request, res: Response): Promise { export async function updateTariff(req: Request, res: Response): Promise { try { - const tariff = await tariffService.updateTariff(parseInt(req.params.id), req.body); + // Pentest R110: Mass-Assignment-Whitelist (nur name/isActive). + const tariff = await tariffService.updateTariff(parseInt(req.params.id), pickTariffUpdate(req.body)); await logChange({ req, action: 'UPDATE', resourceType: 'Tariff', resourceId: tariff.id.toString(), diff --git a/backend/src/utils/sanitize.ts b/backend/src/utils/sanitize.ts index 312da415..02e05e71 100644 --- a/backend/src/utils/sanitize.ts +++ b/backend/src/utils/sanitize.ts @@ -835,3 +835,116 @@ export function pickUserUpdate(body: unknown): Partial> export function pickUserCreate(body: unknown): Partial> { return pick((body as object) || {}, USER_CREATE_FIELDS, { stripHtmlFromStrings: true }); } + +// ==================== KATALOG-/CONFIG-WHITELISTS (Pentest R110) ==================== +// Pentest 2026-07-11 (MEDIUM, R110): sieben Update-Endpunkte reichten +// `req.body` ungefiltert an Prisma durch – gleiches Muster wie das +// bekannte M1-Finding (Settings Mass Assignment). Bewiesen war es via +// `PUT /api/stressfrei-emails/:id`, wo `provisionError` (ausserhalb des +// TS-Types) durchgeschrieben werden konnte. Die anderen sechs (platform, +// tariff, contractCategory, cancellationPeriod, contractDuration, +// email-providers) haben denselben Bug-Pattern. Fix: pro Endpunkt eine +// enge Feld-Whitelist. Nur die Felder aus dem jeweiligen `updateXxx`- +// Service-Interface passieren. +// +// stripHtml auf String-Werten: +// - Für Katalog-/Anzeige-Felder (name, description, code, notes …) OK. +// - Beim EmailProvider bewusst AUS: enthält Passwörter/API-Keys, die +// Sonderzeichen (` `, `<`, `>` etc.) enthalten dürfen. Der Endpunkt +// ist Admin-only, XSS-Risk minimal – wichtiger, dass Passwort nicht +// heimlich mutiliert wird. + +const STRESSFREI_EMAIL_UPDATABLE_FIELDS = [ + 'email', + 'platform', + 'notes', + 'isActive', +] as const; + +const PLATFORM_UPDATABLE_FIELDS = [ + 'name', + 'contactInfo', + 'isActive', +] as const; + +const TARIFF_UPDATABLE_FIELDS = [ + 'name', + 'isActive', +] as const; + +const CONTRACT_CATEGORY_UPDATABLE_FIELDS = [ + 'code', + 'name', + 'icon', + 'color', + 'sortOrder', + 'isActive', +] as const; + +const CANCELLATION_PERIOD_UPDATABLE_FIELDS = [ + 'code', + 'description', + 'isActive', +] as const; + +const CONTRACT_DURATION_UPDATABLE_FIELDS = [ + 'code', + 'description', + 'isActive', +] as const; + +const EMAIL_PROVIDER_UPDATABLE_FIELDS = [ + 'name', + 'type', + 'apiUrl', + 'apiKey', + 'username', + 'password', + 'domain', + 'defaultForwardEmail', + 'imapServer', + 'imapPort', + 'smtpServer', + 'smtpPort', + 'imapEncryption', + 'smtpEncryption', + 'allowSelfSignedCerts', + 'systemEmailAddress', + 'systemEmailPassword', + 'customerEmailLabel', + 'isActive', + 'isDefault', +] as const; + +export function pickStressfreiEmailUpdate(body: unknown): Partial> { + return pick((body as object) || {}, STRESSFREI_EMAIL_UPDATABLE_FIELDS, { stripHtmlFromStrings: true }); +} + +export function pickPlatformUpdate(body: unknown): Partial> { + return pick((body as object) || {}, PLATFORM_UPDATABLE_FIELDS, { stripHtmlFromStrings: true }); +} + +export function pickTariffUpdate(body: unknown): Partial> { + return pick((body as object) || {}, TARIFF_UPDATABLE_FIELDS, { stripHtmlFromStrings: true }); +} + +export function pickContractCategoryUpdate(body: unknown): Partial> { + return pick((body as object) || {}, CONTRACT_CATEGORY_UPDATABLE_FIELDS, { stripHtmlFromStrings: true }); +} + +export function pickCancellationPeriodUpdate(body: unknown): Partial> { + return pick((body as object) || {}, CANCELLATION_PERIOD_UPDATABLE_FIELDS, { stripHtmlFromStrings: true }); +} + +export function pickContractDurationUpdate(body: unknown): Partial> { + return pick((body as object) || {}, CONTRACT_DURATION_UPDATABLE_FIELDS, { stripHtmlFromStrings: true }); +} + +/** + * EmailProvider hat Passwörter/API-Keys unter den Feldern; stripHtml + * würde Sonderzeichen wie `<`/`>` in einem legitimen Passwort mutilieren. + * Whitelist-Filter ja, HTML-Strip nein. + */ +export function pickEmailProviderUpdate(body: unknown): Partial> { + return pick((body as object) || {}, EMAIL_PROVIDER_UPDATABLE_FIELDS); +} diff --git a/docs/SECURITY-HARDENING.md b/docs/SECURITY-HARDENING.md index 6023362a..f7bbfce2 100644 --- a/docs/SECURITY-HARDENING.md +++ b/docs/SECURITY-HARDENING.md @@ -654,6 +654,39 @@ Modus hängt sowieso an einer schon validierten Email-Stammdate --- +## 🔒 Runde 110 – Mass-Assignment-Whitelist auf 7 Katalog-Endpunkten + +**Finding (MEDIUM):** Der Pentester hat live nachgewiesen, dass +`PUT /api/stressfrei-emails/:id` beliebige Model-Felder ausserhalb +des TS-Types (z.B. `provisionError`, `isProvisioned`, +`emailPasswordEncrypted`) durchschrieb. Beim „Rumstochern" derselbe +Bug auf sechs weiteren Update-Endpunkten gefunden: `platform`, +`tariff`, `contractCategory`, `cancellationPeriod`, +`contractDuration`, `email-providers`. Gleiche Bug-Klasse wie +das schon gefixte M1-Finding (Settings Mass Assignment), nur an +sieben weiteren Stellen. + +Praktische Ausnutzung braucht Staff mit der entsprechenden +Update-Permission – kein Portal-User-Vektor, kein Cross-Customer- +Leak. Aber ohne Whitelist könnte ein kompromittierter Staff-Token +z.B. `emailPasswordEncrypted` überschreiben, ohne einen sichtbaren +Audit-Trail durch den regulären Passwort-Set-Flow. + +**Fix:** Sieben Field-Whitelists + `pickXxxUpdate()`-Helper in +[`backend/src/utils/sanitize.ts`](../backend/src/utils/sanitize.ts), +eingehängt in die jeweiligen Update-Controller. Nur die vom +Service-Interface deklarierten Felder passieren. Reuse der schon +bewährten `pick()`-Infrastruktur (Customer/User haben denselben +Mechanismus seit Runde 7). + +**Ausnahme EmailProvider:** `stripHtmlFromStrings` ist dort AUS. +Grund: das Payload enthält Passwörter und API-Keys, die legitime +Sonderzeichen wie `<`/`>` enthalten dürfen. Whitelist-Filter greift +weiterhin – XSS-Risiko minimal, da der Endpunkt admin-only ist und +die Werte in Server-Config-Feldern landen, nicht in User-Notes. + +--- + ## 🔒 Runde 102 – Interne Vertragsnummer nachziehen **Finding (INFO, kein Security-Impact ohne Admin-Login):** diff --git a/docs/todo.md b/docs/todo.md index 5f2526d9..3bcc22b1 100644 --- a/docs/todo.md +++ b/docs/todo.md @@ -97,6 +97,19 @@ isolierte Instanz (keine Multi-Tenancy im Code), Provisioning + Abrechnung ## ✅ Erledigt +- [x] **🔒 Pentest R110 – Mass-Assignment-Whitelist auf 7 Update-Endpunkten** + - MEDIUM-Finding: `PUT /api/stressfrei-emails/:id` und 6 weitere Update- + Endpunkte (`platform`, `tariff`, `contractCategory`, + `cancellationPeriod`, `contractDuration`, `email-providers`) reichten + `req.body` ungefiltert an Prisma – gleiche Bug-Klasse wie M1 + (Settings Mass Assignment). Nachgewiesen war es via + `provisionError`-Feld ausserhalb des TS-Types. + - Fix: sieben Whitelists + `pickXxxUpdate()`-Helper in `sanitize.ts`, + in den jeweiligen Controllern eingehängt. Nur die vom Service- + Interface deklarierten Felder passieren. + - EmailProvider: bewusst ohne `stripHtmlFromStrings`, weil das + Passwörter/API-Keys mit Sonderzeichen mutiliert hätte. + - [x] **🐞 Kündigungsdatum: Cursor sprang beim Tippen aus dem Feld** - `` feuerte `onChange` bei jedem Tastendruck; sobald z.B. `18.08.0002` ein gültiges Datum ergab, feuerte die PUT-Mutation,