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

Pentest 58.1 MEDIUM: documentType jetzt mit echter Whitelist-Validierung

Browse Source 
Bisher lief documentType nur durch stripHtml – ein beliebiger String
("NICHT_ERLAUBT", "DROP TABLE ...", Tippfehler) wurde 1:1 als
ContractDocument.documentType in die DB geschrieben. Das brach
Frontend-Filter, Lieferbestätigung-Auto-Activation und Reports.

Neuer validateContractDocumentType-Helper in utils/sanitize:
- Whitelist ALLOWED_CONTRACT_DOCUMENT_TYPES (8 Werte, gespiegelt aus
  Frontend CONTRACT_DOCUMENT_TYPES)
- Case-insensitiver Match, Rückgabe ist immer der kanonische Wert
- Wirft sprechende 400-Fehlermeldung mit Liste der erlaubten Werte

Eingesetzt in allen 3 Schreibpfaden:
- contract.controller.uploadContractDocument (multer-Datei wird bei
  Reject sauber gelöscht)
- cachedEmail.controller.saveEmailAsContractDocument
- cachedEmail.controller.saveAttachmentAsContractDocument

Audit-Log + maybeActivateOnDeliveryConfirmation nutzen jetzt den
kanonischen Wert (statt der rohen Eingabe), damit Reports
einheitlich aussehen.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
duffyduck
2026-06-01 21:53:34 +02:00
parent 9482424ade
commit 6b1d493f0b
3 changed files with 83 additions and 20 deletions
Download Patch File Download Diff File Expand all files Collapse all files
backend/src/controllers/cachedEmail.controller.ts
+21 -15
View File
@@ -8,7 +8,7 @@ import { sendEmail, SmtpCredentials, SendEmailParams, EmailAttachment } from '..
import { fetchAttachment, appendToSent, ImapCredentials } from '../services/imapService.js';
import { getImapSmtpSettings } from '../services/emailProvider/emailProviderService.js';
import { decrypt } from '../utils/encryption.js';
import { sanitizeNotes, stripHtml } from '../utils/sanitize.js';
import { sanitizeNotes, stripHtml, validateContractDocumentType } from '../utils/sanitize.js';
import { logChange } from '../services/audit.service.js';
import { ApiResponse, AuthRequest } from '../types/index.js';
import { getCustomerTargets, getContractTargets, getIdentityDocumentTargets, getBankCardTargets, documentTargets } from '../config/documentTargets.config.js';
@@ -1827,8 +1827,12 @@ export async function saveEmailAsContractDocument(req: AuthRequest, res: Respons
if (!(await canAccessCachedEmail(req, res, emailId))) return;
const { documentType, notes } = req.body;
if (!documentType || typeof documentType !== 'string') {
res.status(400).json({ success: false, error: 'documentType ist erforderlich' } as ApiResponse);
// Pentest 58.1: Whitelist-Validierung des documentType.
let validatedType: string;
try {
validatedType = validateContractDocumentType(documentType);
} catch (err) {
res.status(400).json({ success: false, error: err instanceof Error ? err.message : 'Ungültiger documentType' } as ApiResponse);
return;
}
@@ -1872,21 +1876,20 @@ export async function saveEmailAsContractDocument(req: AuthRequest, res: Respons
if (!fs.existsSync(uploadsDir)) fs.mkdirSync(uploadsDir, { recursive: true });
const uniqueSuffix = Date.now() + '-' + Math.round(Math.random() * 1e9);
const safeType = documentType.toLowerCase().replace(/[^a-z0-9]/g, '-');
const safeType = validatedType.toLowerCase().replace(/[^a-z0-9]/g, '-');
const newFilename = `${safeType}-email-${uniqueSuffix}.pdf`;
const filePath = path.join(uploadsDir, newFilename);
const relativePath = `/uploads/contract-documents/${newFilename}`;
const cleanType = stripHtml(documentType) as string;
// Pentest 55.4: Lock vor Schreiben, damit parallele Requests blockieren.
// PDF erst INNERHALB des Locks auf Disk schreiben → kein verwaister
// Datei-Müll bei Race-Reject.
const doc = await withContractDocumentLock(contract.id, cleanType, async () => {
const doc = await withContractDocumentLock(contract.id, validatedType, async () => {
fs.writeFileSync(filePath, pdfBuffer);
return prisma.contractDocument.create({
data: {
contractId: contract.id,
documentType: cleanType,
documentType: validatedType,
documentPath: relativePath,
originalName: `${email.subject || 'email'}.pdf`,
notes: sanitizeNotes(notes),
@@ -1897,7 +1900,7 @@ export async function saveEmailAsContractDocument(req: AuthRequest, res: Respons
// Falls Lieferbestätigung: DRAFT → ACTIVE + startDate setzen falls leer
const deliveryDate = typeof req.body?.deliveryDate === 'string' ? req.body.deliveryDate : null;
await maybeActivateOnDeliveryConfirmation(contract.id, documentType, req, deliveryDate);
await maybeActivateOnDeliveryConfirmation(contract.id, validatedType, req, deliveryDate);
res.json({ success: true, data: doc } as ApiResponse);
} catch (error) {
@@ -2081,10 +2084,14 @@ export async function saveAttachmentAsContractDocument(req: AuthRequest, res: Re
const filename = decodeURIComponent(req.params.filename);
const { documentType, notes } = req.body;
if (!documentType || typeof documentType !== 'string') {
// Pentest 58.1: Whitelist-Validierung des documentType.
let validatedType: string;
try {
validatedType = validateContractDocumentType(documentType);
} catch (err) {
res.status(400).json({
success: false,
error: 'documentType ist erforderlich',
error: err instanceof Error ? err.message : 'Ungültiger documentType',
} as ApiResponse);
return;
}
@@ -2174,19 +2181,18 @@ export async function saveAttachmentAsContractDocument(req: AuthRequest, res: Re
const ext = path.extname(filename) || '.pdf';
const uniqueSuffix = Date.now() + '-' + Math.round(Math.random() * 1e9);
const safeType = documentType.toLowerCase().replace(/[^a-z0-9]/g, '-');
const safeType = validatedType.toLowerCase().replace(/[^a-z0-9]/g, '-');
const newFilename = `${safeType}-${uniqueSuffix}${ext}`;
const filePath = path.join(uploadsDir, newFilename);
const relativePath = `/uploads/contract-documents/${newFilename}`;
const cleanType = stripHtml(documentType) as string;
// Pentest 55.4: Lock vor Schreiben (siehe saveEmailAsContractDocument).
const doc = await withContractDocumentLock(contract.id, cleanType, async () => {
const doc = await withContractDocumentLock(contract.id, validatedType, async () => {
fs.writeFileSync(filePath, attachment.content);
return prisma.contractDocument.create({
data: {
contractId: contract.id,
documentType: cleanType,
documentType: validatedType,
documentPath: relativePath,
originalName: filename,
notes: sanitizeNotes(notes),
@@ -2197,7 +2203,7 @@ export async function saveAttachmentAsContractDocument(req: AuthRequest, res: Re
// Falls Lieferbestätigung: DRAFT → ACTIVE + startDate setzen falls leer
const deliveryDate = typeof req.body?.deliveryDate === 'string' ? req.body.deliveryDate : null;
await maybeActivateOnDeliveryConfirmation(contract.id, documentType, req, deliveryDate);
await maybeActivateOnDeliveryConfirmation(contract.id, validatedType, req, deliveryDate);
res.json({ success: true, data: doc } as ApiResponse);
} catch (error) {
backend/src/controllers/contract.controller.ts
+15 -5
View File
@@ -1,4 +1,5 @@
import { Request, Response } from 'express';
import fs from 'fs';
import prisma from '../lib/prisma.js';
import * as contractService from '../services/contract.service.js';
import * as contractCockpitService from '../services/contractCockpit.service.js';
@@ -7,7 +8,7 @@ import * as authorizationService from '../services/authorization.service.js';
import { recordPredecessorFinalReading } from '../services/customer.service.js';
import { ApiResponse, AuthRequest } from '../types/index.js';
import { logChange } from '../services/audit.service.js';
import { sanitizeContract, sanitizeContractStrict, sanitizeContracts, sanitizeContractsStrict, stripHtml, sanitizeNotes } from '../utils/sanitize.js';
import { sanitizeContract, sanitizeContractStrict, sanitizeContracts, sanitizeContractsStrict, stripHtml, sanitizeNotes, validateContractDocumentType } from '../utils/sanitize.js';
import { canAccessContract } from '../utils/accessControl.js';
import { maybeActivateOnDeliveryConfirmation, withContractDocumentLock } from '../services/contractStatusScheduler.service.js';
@@ -734,7 +735,16 @@ export async function uploadContractDocument(req: AuthRequest, res: Response): P
}
const documentPath = `/uploads/contract-documents/${req.file.filename}`;
const cleanType = stripHtml(documentType) as string;
// Pentest 58.1: Whitelist-Validierung statt nur stripHtml. Multer hat
// die Datei schon geschrieben – bei Reject räumen wir sie wieder weg.
let cleanType: string;
try {
cleanType = validateContractDocumentType(documentType);
} catch (err) {
try { fs.unlinkSync(req.file.path); } catch { /* ignore */ }
res.status(400).json({ success: false, error: err instanceof Error ? err.message : 'Ungültiger Dokumenttyp' } as ApiResponse);
return;
}
// Pentest 55.4: Race-Schutz – Lock + Recent-Duplicate-Check.
const doc = await withContractDocumentLock(contractId, cleanType, () =>
prisma.contractDocument.create({
@@ -753,13 +763,13 @@ export async function uploadContractDocument(req: AuthRequest, res: Response): P
await logChange({
req, action: 'CREATE', resourceType: 'ContractDocument',
resourceId: doc.id.toString(),
label: `Dokument "${documentType}" hochgeladen für Vertrag ${contract?.contractNumber}`,
details: { typ: documentType, datei: req.file.originalname },
label: `Dokument "${cleanType}" hochgeladen für Vertrag ${contract?.contractNumber}`,
details: { typ: cleanType, datei: req.file.originalname },
customerId: contract?.customerId,
});
// Falls Lieferbestätigung: DRAFT → ACTIVE + startDate setzen falls leer
await maybeActivateOnDeliveryConfirmation(contractId, documentType, req, deliveryDate);
await maybeActivateOnDeliveryConfirmation(contractId, cleanType, req, deliveryDate);
res.status(201).json({ success: true, data: doc } as ApiResponse);
} catch (error) {
backend/src/utils/sanitize.ts
+47
View File
@@ -178,6 +178,53 @@ export function sanitizeCustomers<T extends Record<string, unknown>>(customers:
// - Tags + gefährliche Schemata via stripHtml
// - CRLF auf Newline normalisieren, keine Carriage-Returns persistieren
// - Length-Cap default 2000 Zeichen (genug für sinnvolle Anmerkungen)
// Pentest 58.1 (MEDIUM, 2026-06-01): documentType wurde nur durch
// stripHtml geschickt, aber NICHT gegen eine Whitelist geprüft. Damit
// landeten beliebige Strings (`NICHT_ERLAUBT`, `DROP TABLE …`,
// Tippfehler-Werte aus alten UI-Versionen) als documentType in der
// ContractDocument-Tabelle und brachen Frontend-Filter, Auto-Activation
// (Lieferbestätigung-Trigger) und Reports.
//
// Whitelist spiegelt die Konstante CONTRACT_DOCUMENT_TYPES aus
// SaveAttachmentModal / SaveEmailAsPdfModal im Frontend. Beide
// Listen MÜSSEN synchron gehalten werden – idealerweise später
// in eine geteilte Konfiguration gehoben.
export const ALLOWED_CONTRACT_DOCUMENT_TYPES = [
'Auftragsformular',
'Auftragsbestätigung',
'Lieferbestätigung',
'Vertragsunterlagen',
'Vollmacht',
'Widerrufsbelehrung',
'Preisblatt',
'Sonstiges',
] as const;
const CONTRACT_DOCUMENT_TYPE_SET: Set<string> = new Set(ALLOWED_CONTRACT_DOCUMENT_TYPES);
/**
* Validiert + normalisiert einen documentType-Wert. Wirft einen Fehler
* mit klarer Liste, wenn der Wert nicht in der Whitelist steht (der
* aufrufende Controller mappt das auf 400). Trimmt Whitespace und macht
* den Vergleich case-insensitive – damit `"lieferbestätigung"` aus
* Drittsystemen sauber matched, aber `"Lieferbestätigung_DROP"` rausfliegt.
*/
export function validateContractDocumentType(raw: unknown): string {
if (typeof raw !== 'string') {
throw new Error(`documentType ist erforderlich. Erlaubt: ${ALLOWED_CONTRACT_DOCUMENT_TYPES.join(', ')}`);
}
const cleaned = stripHtml(raw) as string;
const trimmed = cleaned.trim();
if (trimmed === '') {
throw new Error(`documentType ist erforderlich. Erlaubt: ${ALLOWED_CONTRACT_DOCUMENT_TYPES.join(', ')}`);
}
const canonical = ALLOWED_CONTRACT_DOCUMENT_TYPES.find((t) => t.toLowerCase() === trimmed.toLowerCase());
if (!canonical) {
throw new Error(`Ungültiger documentType '${trimmed}'. Erlaubt: ${ALLOWED_CONTRACT_DOCUMENT_TYPES.join(', ')}`);
}
return canonical;
}
const NOTES_DEFAULT_MAX = 2000;
export function sanitizeNotes(raw: unknown, maxLength: number = NOTES_DEFAULT_MAX): string | null {
if (raw == null) return null;