Hacker-Software/opencrm
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Pentest R110: Mass-Assignment-Whitelist auf 7 Katalog-Endpunkten

Browse Source 
MEDIUM: 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 das gefixte M1-Finding, sieben
Stellen mehr. Nachgewiesen via provisionError-Feld ausserhalb des
TS-Types.

Fix: sieben Whitelists + pickXxxUpdate()-Helper in sanitize.ts,
in den jeweiligen Controllern eingehängt. Reuse der bewährten
pick()-Infrastruktur (Customer/User seit Runde 7).

EmailProvider bewusst OHNE stripHtmlFromStrings, weil Passwörter
und API-Keys legitim Sonderzeichen enthalten dürfen.

Doku: SECURITY-HARDENING.md § Runde 110 + docs/todo.md.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
duffyduck
2026-07-11 16:16:49 +02:00
co-authored by Claude Opus 4.7
parent dc7dcfb5f3
commit da3ae282fc
10 changed files with 185 additions and 7 deletions
Download Patch File Download Diff File Expand all files Collapse all files
backend/src/controllers/cancellation-period.controller.ts
+3 -1
View File
@@ -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<void> {
try {
@@ -54,7 +55,8 @@ export async function createCancellationPeriod(req: Request, res: Response): Pro
export async function updateCancellationPeriod(req: Request, res: Response): Promise<void> {
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(),
backend/src/controllers/contract-duration.controller.ts
+3 -1
View File
@@ -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<void> {
try {
@@ -54,7 +55,8 @@ export async function createContractDuration(req: Request, res: Response): Promi
export async function updateContractDuration(req: Request, res: Response): Promise<void> {
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(),
backend/src/controllers/contractCategory.controller.ts
+3 -1
View File
@@ -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<void> {
try {
@@ -54,7 +55,8 @@ export async function createContractCategory(req: Request, res: Response): Promi
export async function updateContractCategory(req: Request, res: Response): Promise<void> {
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(),
backend/src/controllers/emailProvider.controller.ts
+4 -1
View File
@@ -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<void> {
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(),
backend/src/controllers/platform.controller.ts
+3 -1
View File
@@ -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<void> {
try {
@@ -54,7 +55,8 @@ export async function createPlatform(req: Request, res: Response): Promise<void>
export async function updatePlatform(req: Request, res: Response): Promise<void> {
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(),
backend/src/controllers/stressfreiEmail.controller.ts
+7 -1
View File
@@ -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<void
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);
// Pentest R110 (MEDIUM, 2026-07-11): Mass-Assignment ohne Whitelist.
// `req.body` reichte ungefiltert an Prisma – Angreifer konnte
// beliebige Model-Felder (`provisionError`, `isProvisioned`,
// `emailPasswordEncrypted` …) durch. Jetzt strikt auf die vier
// vom Service akzeptierten Felder gefiltert.
const email = await stressfreiEmailService.updateEmail(emailId, pickStressfreiEmailUpdate(req.body));
await logChange({
req, action: 'UPDATE', resourceType: 'StressfreiEmail',
resourceId: email.id.toString(),
backend/src/controllers/tariff.controller.ts
+3 -1
View File
@@ -2,6 +2,7 @@ import { Request, Response } from 'express';
import * as tariffService from '../services/tariff.service.js';
import { logChange } from '../services/audit.service.js';
import { ApiResponse } from '../types/index.js';
import { pickTariffUpdate } from '../utils/sanitize.js';
export async function getTariffs(req: Request, res: Response): Promise<void> {
try {
@@ -56,7 +57,8 @@ export async function createTariff(req: Request, res: Response): Promise<void> {
export async function updateTariff(req: Request, res: Response): Promise<void> {
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(),
backend/src/utils/sanitize.ts
+113
View File
@@ -835,3 +835,116 @@ export function pickUserUpdate(body: unknown): Partial<Record<string, unknown>>
export function pickUserCreate(body: unknown): Partial<Record<string, unknown>> {
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<Record<string, unknown>> {
return pick((body as object) || {}, STRESSFREI_EMAIL_UPDATABLE_FIELDS, { stripHtmlFromStrings: true });
}
export function pickPlatformUpdate(body: unknown): Partial<Record<string, unknown>> {
return pick((body as object) || {}, PLATFORM_UPDATABLE_FIELDS, { stripHtmlFromStrings: true });
}
export function pickTariffUpdate(body: unknown): Partial<Record<string, unknown>> {
return pick((body as object) || {}, TARIFF_UPDATABLE_FIELDS, { stripHtmlFromStrings: true });
}
export function pickContractCategoryUpdate(body: unknown): Partial<Record<string, unknown>> {
return pick((body as object) || {}, CONTRACT_CATEGORY_UPDATABLE_FIELDS, { stripHtmlFromStrings: true });
}
export function pickCancellationPeriodUpdate(body: unknown): Partial<Record<string, unknown>> {
return pick((body as object) || {}, CANCELLATION_PERIOD_UPDATABLE_FIELDS, { stripHtmlFromStrings: true });
}
export function pickContractDurationUpdate(body: unknown): Partial<Record<string, unknown>> {
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<Record<string, unknown>> {
return pick((body as object) || {}, EMAIL_PROVIDER_UPDATABLE_FIELDS);
}
docs/SECURITY-HARDENING.md
+33
View File
@@ -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):**
docs/todo.md
+13
View File
@@ -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**
- `<input type=date>` feuerte `onChange` bei jedem Tastendruck; sobald
z.B. `18.08.0002` ein gültiges Datum ergab, feuerte die PUT-Mutation,