diff --git a/backend/src/controllers/cachedEmail.controller.ts b/backend/src/controllers/cachedEmail.controller.ts index 53ff44c0..6d1b7fed 100644 --- a/backend/src/controllers/cachedEmail.controller.ts +++ b/backend/src/controllers/cachedEmail.controller.ts @@ -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, validateContractDocumentType, validateOptionalIsoDate } from '../utils/sanitize.js'; +import { sanitizeNotes, stripHtml, validateContractDocumentType, validateOptionalIsoDate, assertSafePdf } from '../utils/sanitize.js'; import { ApiError } from '../utils/apiError.js'; import { logChange } from '../services/audit.service.js'; import { ApiResponse, AuthRequest } from '../types/index.js'; @@ -1283,6 +1283,9 @@ export async function saveAttachmentTo(req: AuthRequest, res: Response): Promise const filePath = path.join(uploadsDir, newFilename); const relativePath = `/uploads/${targetDir}/${newFilename}`; + // Pentest 68.1: PDF-Anhänge auf aktive Inhalte scannen (JS/Launch/Embed). + assertSafePdf(attachment.content); + // Datei speichern fs.writeFileSync(filePath, attachment.content); @@ -1433,8 +1436,9 @@ export async function saveAttachmentTo(req: AuthRequest, res: Response): Promise } catch (error) { console.error('saveAttachmentTo error:', error); // Detailliertere Fehlermeldung für Debugging + const status = error instanceof ApiError ? error.statusCode : 500; const errorMessage = error instanceof Error ? error.message : 'Unbekannter Fehler'; - res.status(500).json({ + res.status(status).json({ success: false, error: `Fehler beim Speichern des Anhangs: ${errorMessage}`, } as ApiResponse); @@ -2054,6 +2058,9 @@ export async function saveAttachmentAsInvoice(req: AuthRequest, res: Response): const filePath = path.join(uploadsDir, newFilename); const relativePath = `/uploads/invoices/${newFilename}`; + // Pentest 68.1: PDF-Anhänge auf aktive Inhalte scannen. + assertSafePdf(attachment.content); + // Datei speichern fs.writeFileSync(filePath, attachment.content); @@ -2078,8 +2085,9 @@ export async function saveAttachmentAsInvoice(req: AuthRequest, res: Response): } as ApiResponse); } catch (error) { console.error('saveAttachmentAsInvoice error:', error); + const status = error instanceof ApiError ? error.statusCode : 500; const errorMessage = error instanceof Error ? error.message : 'Unbekannter Fehler'; - res.status(500).json({ + res.status(status).json({ success: false, error: `Fehler beim Erstellen der Rechnung: ${errorMessage}`, } as ApiResponse); @@ -2208,6 +2216,9 @@ export async function saveAttachmentAsContractDocument(req: AuthRequest, res: Re const filePath = path.join(uploadsDir, newFilename); const relativePath = `/uploads/contract-documents/${newFilename}`; + // Pentest 68.1: PDF-Anhänge auf aktive Inhalte scannen. + assertSafePdf(attachment.content); + // Pentest 55.4: Lock vor Schreiben (siehe saveEmailAsContractDocument). const doc = await withContractDocumentLock(contract.id, validatedType, async () => { fs.writeFileSync(filePath, attachment.content); diff --git a/backend/src/middleware/pdfUploadSafety.ts b/backend/src/middleware/pdfUploadSafety.ts new file mode 100644 index 00000000..8d96ae14 --- /dev/null +++ b/backend/src/middleware/pdfUploadSafety.ts @@ -0,0 +1,62 @@ +import { Request, Response, NextFunction } from 'express'; +import fs from 'fs'; +import { assertSafePdf } from '../utils/sanitize.js'; +import { ApiError } from '../utils/apiError.js'; + +/** + * Express-Middleware nach multer.single(...): wenn die abgelegte Datei + * eine PDF ist (Magic-Byte %PDF-), wird sie auf gefährliche aktive + * Inhalte (JS / Launch / EmbeddedFile / RichMedia) gescannt. Bei + * Verstoß: Datei vom Disk löschen + JSON-Error zurückgeben. Non-PDF- + * Dateien passieren ohne Validierung – diese Middleware ist NICHT der + * Magic-Byte-Check für andere Typen. + * + * Pentest 68.1 (LOW, 2026-06-03): Routen, die PDFs annehmen + * (gdpr.routes Vollmacht, contract.routes Vertragsdokumente, + * pdfTemplate.routes) haben bisher nur den client-gemeldeten mimetype + * geprüft; gefährliche PDFs kamen durch. + */ +export function scanUploadedPdfIfPresent(req: Request, res: Response, next: NextFunction): void { + const file = (req as Request & { file?: Express.Multer.File }).file; + if (!file) { + next(); + return; + } + try { + const buf = fs.readFileSync(file.path); + if (buf.length >= 5 && buf.subarray(0, 5).toString('latin1') === '%PDF-') { + assertSafePdf(buf); + } + next(); + } catch (e) { + try { fs.unlinkSync(file.path); } catch { /* ignore */ } + const status = e instanceof ApiError ? e.statusCode : 415; + const message = e instanceof Error ? e.message : 'PDF ungültig'; + res.status(status).json({ success: false, error: message }); + } +} + +/** + * Strikte Variante: Datei MUSS eine PDF sein. Sonst 415. Für Routen, die + * ausschliesslich PDFs zulassen (z.B. Vollmacht-Upload). + */ +export function requireSafeUploadedPdf(req: Request, res: Response, next: NextFunction): void { + const file = (req as Request & { file?: Express.Multer.File }).file; + if (!file) { + next(); + return; + } + try { + const buf = fs.readFileSync(file.path); + if (buf.length < 5 || buf.subarray(0, 5).toString('latin1') !== '%PDF-') { + throw new ApiError(415, 'Datei ist keine gültige PDF.'); + } + assertSafePdf(buf); + next(); + } catch (e) { + try { fs.unlinkSync(file.path); } catch { /* ignore */ } + const status = e instanceof ApiError ? e.statusCode : 415; + const message = e instanceof Error ? e.message : 'PDF ungültig'; + res.status(status).json({ success: false, error: message }); + } +} diff --git a/backend/src/routes/contract.routes.ts b/backend/src/routes/contract.routes.ts index 020f6570..d1807d0a 100644 --- a/backend/src/routes/contract.routes.ts +++ b/backend/src/routes/contract.routes.ts @@ -5,6 +5,7 @@ import fs from 'fs'; import * as contractController from '../controllers/contract.controller.js'; import * as invoiceController from '../controllers/invoice.controller.js'; import { authenticate, requirePermission } from '../middleware/auth.js'; +import { scanUploadedPdfIfPresent } from '../middleware/pdfUploadSafety.js'; const router = Router(); @@ -54,7 +55,7 @@ router.post('/:id/invoices', authenticate, requirePermission('contracts:update') // Vertragsdokumente router.get('/:id/documents', authenticate, requirePermission('contracts:read'), contractController.getContractDocuments); -router.post('/:id/documents', authenticate, requirePermission('contracts:update'), docUpload.single('file'), contractController.uploadContractDocument); +router.post('/:id/documents', authenticate, requirePermission('contracts:update'), docUpload.single('file'), scanUploadedPdfIfPresent, contractController.uploadContractDocument); router.delete('/:id/documents/:documentId', authenticate, requirePermission('contracts:update'), contractController.deleteContractDocument); // Folgezähler diff --git a/backend/src/routes/gdpr.routes.ts b/backend/src/routes/gdpr.routes.ts index 31e191c4..804594a2 100644 --- a/backend/src/routes/gdpr.routes.ts +++ b/backend/src/routes/gdpr.routes.ts @@ -3,6 +3,7 @@ import multer from 'multer'; import path from 'path'; import fs from 'fs'; import { authenticate, requirePermission } from '../middleware/auth.js'; +import { requireSafeUploadedPdf } from '../middleware/pdfUploadSafety.js'; import * as gdprController from '../controllers/gdpr.controller.js'; const router = Router(); @@ -84,7 +85,7 @@ router.get('/customer/:customerId/authorizations', requirePermission('customers: router.post('/customer/:customerId/authorizations/:representativeId/send', requirePermission('customers:update'), gdprController.sendAuthorizationRequest); router.post('/customer/:customerId/authorizations/:representativeId/grant', requirePermission('customers:update'), gdprController.grantAuthorization); router.post('/customer/:customerId/authorizations/:representativeId/withdraw', requirePermission('customers:update'), gdprController.withdrawAuthorization); -router.post('/customer/:customerId/authorizations/:representativeId/upload', requirePermission('customers:update'), authUpload.single('document'), gdprController.uploadAuthorizationDocument); +router.post('/customer/:customerId/authorizations/:representativeId/upload', requirePermission('customers:update'), authUpload.single('document'), requireSafeUploadedPdf, gdprController.uploadAuthorizationDocument); router.delete('/customer/:customerId/authorizations/:representativeId/document', requirePermission('customers:update'), gdprController.deleteAuthorizationDocument); // Portal: Vollmachten diff --git a/backend/src/routes/pdfTemplate.routes.ts b/backend/src/routes/pdfTemplate.routes.ts index 0f003c66..c9ce13bd 100644 --- a/backend/src/routes/pdfTemplate.routes.ts +++ b/backend/src/routes/pdfTemplate.routes.ts @@ -3,6 +3,7 @@ import multer from 'multer'; import path from 'path'; import fs from 'fs'; import { authenticate, requirePermission } from '../middleware/auth.js'; +import { requireSafeUploadedPdf } from '../middleware/pdfUploadSafety.js'; import * as pdfTemplateController from '../controllers/pdfTemplate.controller.js'; const router = Router(); @@ -34,7 +35,7 @@ router.use(authenticate); router.get('/', requirePermission('settings:read'), pdfTemplateController.getTemplates); router.get('/crm-fields', requirePermission('settings:read'), pdfTemplateController.getCrmFields); router.get('/:id', requirePermission('settings:read'), pdfTemplateController.getTemplate); -router.post('/', requirePermission('settings:update'), upload.single('template'), pdfTemplateController.createTemplate); +router.post('/', requirePermission('settings:update'), upload.single('template'), requireSafeUploadedPdf, pdfTemplateController.createTemplate); router.put('/:id', requirePermission('settings:update'), pdfTemplateController.updateTemplate); router.delete('/:id', requirePermission('settings:update'), pdfTemplateController.deleteTemplate); diff --git a/backend/src/routes/upload.routes.ts b/backend/src/routes/upload.routes.ts index 1cf973a8..b0f3da5a 100644 --- a/backend/src/routes/upload.routes.ts +++ b/backend/src/routes/upload.routes.ts @@ -12,7 +12,8 @@ import { canAccessBankCard, canAccessIdentityDocument, } from '../utils/accessControl.js'; -import { validateOptionalIsoDate } from '../utils/sanitize.js'; +import { validateOptionalIsoDate, assertSafePdf } from '../utils/sanitize.js'; +import { ApiError } from '../utils/apiError.js'; // Pentest 56.1 (HIGH, 2026-06-01): Upload-Endpoints prüften nur die // Permission, nicht ob die Ziel-Resource zum Caller passt. Helper-Funktion @@ -140,6 +141,20 @@ function validateUploadedFile(req: AuthRequest, res: Response, next: Function) { return; } + // Pentest 68.1 (LOW): PDF-Body auf aktive Inhalte scannen. + if (detected.mime === 'application/pdf') { + try { + const fullBuf = fs.readFileSync(req.file.path); + assertSafePdf(fullBuf); + } catch (e) { + try { fs.unlinkSync(req.file.path); } catch { /* ignore */ } + const status = e instanceof ApiError ? e.statusCode : 415; + const msg = e instanceof Error ? e.message : 'PDF ungültig'; + res.status(status).json({ success: false, error: msg }); + return; + } + } + // Filename auf kanonische Extension normalisieren. Multer hat // `.gif.php` o.ä. geschrieben – wir wollen `.gif`. const dir = path.dirname(req.file.path); diff --git a/backend/src/utils/sanitize.ts b/backend/src/utils/sanitize.ts index 2f4288a2..109abc93 100644 --- a/backend/src/utils/sanitize.ts +++ b/backend/src/utils/sanitize.ts @@ -4,6 +4,8 @@ * Verschlüsselungen oder Reset-Tokens versehentlich durch die API leaken. */ +import { ApiError } from './apiError.js'; + // Felder die NIE in einer API-Response an den Client gehen dürfen const SENSITIVE_CUSTOMER_FIELDS = [ 'portalPasswordHash', @@ -246,6 +248,45 @@ export function assertValidDocumentPath(v: string | null | undefined, fieldLabel } } +// Pentest 68.1 (LOW, 2026-06-03): PDFs mit JavaScript, /Launch (externes +// Programm), /EmbeddedFile (eingebettete Executables) oder /RichMedia +// (Flash) kamen durch den reinen Magic-Byte-Check (%PDF-) und wurden +// inline ausgeliefert. Browser-PDF-Viewer (PDFium/PDF.js) führen kein JS +// aus, aber sobald jemand die PDF in Adobe Acrobat öffnet, läuft sie. +// → Wir blocken das schon beim Upload. +// +// PDF-Name-Objekte sind laut PDF 32000-1:2008 §7.3.5 case-sensitive, also +// kein /i auf den Patterns. Whitespace nach `/` ist im Standard zwar +// erlaubt, in real-world Exploits aber praktisch nie zu sehen – wir +// bleiben hier pragmatisch. +// +// Hinweis: erkannt wird nur, was im Klartext im PDF-Body steht. +// Komprimierte oder verschlüsselte Streams entgehen dem String-Scan. +// Für unser Bedrohungsmodell (kompromittierter Staff-Account, LOW) reicht +// das – ein vollständiger PDF-Parser wäre Overkill. +const PDF_DANGER_PATTERNS: { pattern: RegExp; label: string }[] = [ + { pattern: /\/JavaScript\b/, label: 'JavaScript-Action' }, + { pattern: /\/JS\b/, label: 'JavaScript-Action' }, + { pattern: /\/Launch\b/, label: 'Launch-Action (externes Programm)' }, + { pattern: /\/EmbeddedFile\b/, label: 'eingebettete Datei' }, + { pattern: /\/RichMedia\b/, label: 'RichMedia-Inhalt (Flash)' }, +]; + +export function assertSafePdf(buf: Buffer): void { + if (buf.length < 5 || buf.subarray(0, 5).toString('latin1') !== '%PDF-') { + return; // keine PDF → andere Validatoren zuständig + } + const content = buf.toString('latin1'); + for (const { pattern, label } of PDF_DANGER_PATTERNS) { + if (pattern.test(content)) { + throw new ApiError( + 415, + `PDF enthält nicht erlaubte aktive Inhalte (${label}). Bitte ohne JavaScript / Auto-Actions / eingebettete Dateien hochladen.`, + ); + } + } +} + // Pentest 51.3 + 60.3 (MEDIUM, 2026-06-01): Telefon-/Vorwahl-Felder // dürfen NIE CRLF oder andere Control-Chars enthalten – sonst sind sie // ein Header-Injection-Vektor (Mail, HTTP), wenn der Wert mal in einen diff --git a/docs/todo.md b/docs/todo.md index 0f1c879d..5e005cfc 100644 --- a/docs/todo.md +++ b/docs/todo.md @@ -97,6 +97,37 @@ isolierte Instanz (keine Multi-Tenancy im Code), Provisioning + Abrechnung ## ✅ Erledigt +- [x] **🔒 Pentest 68.1 (LOW) + 68.2 (INFO): PDF-Inhalts-Validierung + Modal-Limit** + - **68.1 PDF-Active-Content-Filter:** Magic-Byte-Check prüfte bisher + nur `%PDF-`. PDFs mit `/JavaScript`, `/JS`, `/Launch` (externes + Programm), `/EmbeddedFile`, `/RichMedia` (Flash) wurden inline an + den Viewer ausgeliefert – Browser-PDF-Viewer (Chrome/Firefox) + ignorieren JS, Adobe Acrobat aber nicht. + - Neuer Helper `assertSafePdf(buf)` in `utils/sanitize.ts`: + String-Scan auf die fünf Action-Pattern (case-sensitive nach + PDF 32000-1:2008 §7.3.5). Wirft `ApiError(415, ...)` bei Treffer. + - Neue Middleware `pdfUploadSafety.ts` mit zwei Varianten: + - `requireSafeUploadedPdf` – Datei MUSS PDF sein, sonst 415. + - `scanUploadedPdfIfPresent` – durchwinkt JPG/PNG, scannt nur PDFs. + - Eingehängt: + - `upload.routes.ts` (Magic-Byte-Validator erweitert) + - `gdpr.routes.ts` Vollmacht-Upload + - `pdfTemplate.routes.ts` Template-Upload + - `contract.routes.ts` Vertragsdokumente + - `cachedEmail.controller.ts` Email-Anhang-Pfade (3 Stellen: + saveAttachmentTo, saveAttachmentAsInvoice, + saveAttachmentAsContractDocument) + - **Inline-Vorschau bleibt erhalten** – das war die explizite + Anforderung (Augen-Button öffnet PDF im neuen Tab). Pentester- + Empfehlung „disposition=inline abschalten" wurde bewusst NICHT + umgesetzt, weil sie das eigentliche Acrobat-Risiko nicht löst + (PDF auf Disk + Doppelklick → Acrobat → JS läuft trotzdem). + - Edge-Case-Test bestätigt: `/JSXForm` und `/JavaScriptFooter` werden + NICHT als JavaScript-Action erkannt (word-boundary `\b` greift). + - **68.2 Modal-Limit:** `JpgToPdfModal` hatte kein Bild-/Größen-Limit. + Jetzt `MAX_IMAGES = 50` + `MAX_IMAGE_BYTES = 25 MB` pro Bild. + UX-Schutz, kein Security-Bug (Self-DoS only). + - [x] **🆕 JPGs → PDF: Button überall bei PDF-Upload** - Neue Komponente `JpgToPdfModal` (lokal im Browser via `jspdf`, keine Backend-Round-Trip nötig). Mehrere Bilder hinzufügen per diff --git a/frontend/src/components/ui/JpgToPdfModal.tsx b/frontend/src/components/ui/JpgToPdfModal.tsx index f4e97811..c1fb141f 100644 --- a/frontend/src/components/ui/JpgToPdfModal.tsx +++ b/frontend/src/components/ui/JpgToPdfModal.tsx @@ -31,6 +31,12 @@ interface JpgToPdfModalProps { fileNameHint?: string; } +// Pentest 68.2 (INFO): Self-DoS-Schutz – Modal kann sonst den Tab des +// Uploaders selbst zum Absturz bringen. Werte konservativ gewählt: +// 50 Bilder × 25 MB = 1.25 GB ist mehr als jede legitime Vollmacht. +const MAX_IMAGES = 50; +const MAX_IMAGE_BYTES = 25 * 1024 * 1024; + function makeId() { return `${Date.now()}-${Math.random().toString(36).slice(2, 9)}`; } @@ -107,6 +113,15 @@ export default function JpgToPdfModal({ setError(null); const added: ImageItem[] = []; for (const file of list) { + // 68.2: Self-DoS-Schutz – harte Schranken pro Bild und gesamt. + if (file.size > MAX_IMAGE_BYTES) { + setError(`Bild zu groß (max. ${Math.round(MAX_IMAGE_BYTES / 1024 / 1024)} MB): ${file.name || 'unbenannt'}`); + continue; + } + if (images.length + added.length >= MAX_IMAGES) { + setError(`Maximal ${MAX_IMAGES} Bilder pro PDF erlaubt.`); + break; + } try { const dataUrl = await readFileAsDataUrl(file); const img = await loadImage(dataUrl); @@ -127,7 +142,7 @@ export default function JpgToPdfModal({ if (added.length > 0) { setImages((prev) => [...prev, ...added]); } - }, []); + }, [images.length]); useEffect(() => { if (!isOpen) return;