diff --git a/backend/src/middleware/pdfUploadSafety.ts b/backend/src/middleware/pdfUploadSafety.ts index 8d96ae14..3282933d 100644 --- a/backend/src/middleware/pdfUploadSafety.ts +++ b/backend/src/middleware/pdfUploadSafety.ts @@ -3,42 +3,14 @@ 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). + * ausschliesslich PDFs zulassen (z.B. Vollmacht-Upload, PDF-Templates). + * + * Routen, die auch JPG/PNG akzeptieren (z.B. contract.routes + * Vertragsdokumente), nutzen `validateUploadedFile` aus + * `uploadFileTypeValidator.ts` – das macht Magic-Byte für ALLE Typen + + * PDF-Scan in einer Pipeline. */ export function requireSafeUploadedPdf(req: Request, res: Response, next: NextFunction): void { const file = (req as Request & { file?: Express.Multer.File }).file; diff --git a/backend/src/middleware/uploadFileTypeValidator.ts b/backend/src/middleware/uploadFileTypeValidator.ts new file mode 100644 index 00000000..1883ef9d --- /dev/null +++ b/backend/src/middleware/uploadFileTypeValidator.ts @@ -0,0 +1,98 @@ +import { Request, Response, NextFunction } from 'express'; +import fs from 'fs'; +import path from 'path'; +import { assertSafePdf } from '../utils/sanitize.js'; +import { ApiError } from '../utils/apiError.js'; + +/** + * Magic-Byte-Whitelist + canonical Extension Rename + PDF-Active-Content- + * Scan in einer Middleware. Greift nach `multer.single(...)`, prüft die + * geschriebene Datei auf erlaubten Typ (PDF/JPG/PNG/GIF/WebP) und benennt + * sie auf eine kanonische Endung um – damit verschwindet die + * `evil.gif.php`-Doppel-Endung und der client-gemeldete mimetype wird + * durch den ERKANNTEN ersetzt. + * + * Historie: + * - Pentest 39.3 / 39.4 (2026-05-30): Magic-Byte + canonical Rename. + * - Pentest 68.1 (2026-06-03): PDF-Body-Scan auf JS/Launch/Embed/RichMedia. + * - Pentest 69.3 (2026-06-03): Wiederverwendung in contract.routes.ts + * (Vertragsdokumente) – vorher waren JPG/PNG-Uploads dort ungeprüft, + * nur durch Download-Layer kompensiert. + */ + +const PDF_MAGIC = Buffer.from('%PDF-', 'latin1'); +const PNG_MAGIC = Buffer.from([0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a]); +const GIF87 = Buffer.from('GIF87a', 'latin1'); +const GIF89 = Buffer.from('GIF89a', 'latin1'); + +export function detectFileType(buf: Buffer): { mime: string; ext: string } | null { + if (buf.length >= 5 && buf.subarray(0, 5).equals(PDF_MAGIC)) return { mime: 'application/pdf', ext: '.pdf' }; + if (buf.length >= 8 && buf.subarray(0, 8).equals(PNG_MAGIC)) return { mime: 'image/png', ext: '.png' }; + if (buf.length >= 3 && buf[0] === 0xff && buf[1] === 0xd8 && buf[2] === 0xff) return { mime: 'image/jpeg', ext: '.jpg' }; + if (buf.length >= 6 && (buf.subarray(0, 6).equals(GIF87) || buf.subarray(0, 6).equals(GIF89))) return { mime: 'image/gif', ext: '.gif' }; + if (buf.length >= 12 + && buf.subarray(0, 4).toString('latin1') === 'RIFF' + && buf.subarray(8, 12).toString('latin1') === 'WEBP') return { mime: 'image/webp', ext: '.webp' }; + return null; +} + +export function validateUploadedFile(req: Request, res: Response, next: NextFunction): void { + const file = (req as Request & { file?: Express.Multer.File }).file; + if (!file) { + next(); + return; + } + try { + const fd = fs.openSync(file.path, 'r'); + const head = Buffer.alloc(12); + fs.readSync(fd, head, 0, 12, 0); + fs.closeSync(fd); + + const detected = detectFileType(head); + if (!detected) { + try { fs.unlinkSync(file.path); } catch { /* ignore */ } + res.status(415).json({ + success: false, + error: 'Datei-Inhalt entspricht keinem zulässigen Typ (PDF, JPG, PNG, GIF, WebP).', + }); + return; + } + + if (detected.mime === 'application/pdf') { + try { + const fullBuf = fs.readFileSync(file.path); + assertSafePdf(fullBuf); + } catch (e) { + try { fs.unlinkSync(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; + } + } + + const dir = path.dirname(file.path); + const base = path.basename(file.path).replace(/\.[^./]+(\.[^./]+)*$/, ''); + const newName = base + detected.ext; + const newPath = path.join(dir, newName); + if (newPath !== file.path) { + try { + fs.renameSync(file.path, newPath); + file.path = newPath; + file.filename = newName; + } catch (e) { + try { fs.unlinkSync(file.path); } catch { /* ignore */ } + console.error('Upload-Rename fehlgeschlagen:', e); + res.status(500).json({ success: false, error: 'Upload konnte nicht abgeschlossen werden' }); + return; + } + } + + file.mimetype = detected.mime; + next(); + } catch (e) { + console.error('Magic-Byte-Check fehlgeschlagen:', e); + try { fs.unlinkSync(file.path); } catch { /* ignore */ } + res.status(500).json({ success: false, error: 'Upload konnte nicht geprüft werden' }); + } +} diff --git a/backend/src/routes/contract.routes.ts b/backend/src/routes/contract.routes.ts index d1807d0a..6e183381 100644 --- a/backend/src/routes/contract.routes.ts +++ b/backend/src/routes/contract.routes.ts @@ -5,7 +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'; +import { validateUploadedFile } from '../middleware/uploadFileTypeValidator.js'; const router = Router(); @@ -55,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'), scanUploadedPdfIfPresent, contractController.uploadContractDocument); +router.post('/:id/documents', authenticate, requirePermission('contracts:update'), docUpload.single('file'), validateUploadedFile, contractController.uploadContractDocument); router.delete('/:id/documents/:documentId', authenticate, requirePermission('contracts:update'), contractController.deleteContractDocument); // Folgezähler diff --git a/backend/src/routes/upload.routes.ts b/backend/src/routes/upload.routes.ts index b0f3da5a..bbf76fd2 100644 --- a/backend/src/routes/upload.routes.ts +++ b/backend/src/routes/upload.routes.ts @@ -12,8 +12,8 @@ import { canAccessBankCard, canAccessIdentityDocument, } from '../utils/accessControl.js'; -import { validateOptionalIsoDate, assertSafePdf } from '../utils/sanitize.js'; -import { ApiError } from '../utils/apiError.js'; +import { validateOptionalIsoDate } from '../utils/sanitize.js'; +import { validateUploadedFile } from '../middleware/uploadFileTypeValidator.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 @@ -95,98 +95,9 @@ function setUploadDir(subDir: string) { }; } -/** - * Post-Upload-Validierung: prüft die Magic-Bytes der gerade geschriebenen - * Datei und vergleicht mit dem fileFilter-Whitelist. Bei Mismatch - * (Pentest 2026-05-30 LOW 39.3: WebP/GIF/JPG/PDF-Spoofing) wird die - * Datei sofort gelöscht + 415 zurück. - * - * Zusätzlich (39.4): die Datei wird auf eine kanonische Endung umbenannt, - * die aus dem ERKANNTEN Typ abgeleitet ist – nicht aus dem - * client-gemeldeten file.originalname. Damit verschwindet die - * `evil.gif.php`-Doppel-Endung; gespeicherter Name ist - * `.` (z.B. `.pdf` / `.png`). - */ -const PDF_MAGIC = Buffer.from('%PDF-', 'latin1'); -const PNG_MAGIC = Buffer.from([0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a]); -const GIF87 = Buffer.from('GIF87a', 'latin1'); -const GIF89 = Buffer.from('GIF89a', 'latin1'); - -function detectType(buf: Buffer): { mime: string; ext: string } | null { - if (buf.length >= 5 && buf.subarray(0, 5).equals(PDF_MAGIC)) return { mime: 'application/pdf', ext: '.pdf' }; - if (buf.length >= 8 && buf.subarray(0, 8).equals(PNG_MAGIC)) return { mime: 'image/png', ext: '.png' }; - if (buf.length >= 3 && buf[0] === 0xff && buf[1] === 0xd8 && buf[2] === 0xff) return { mime: 'image/jpeg', ext: '.jpg' }; - if (buf.length >= 6 && (buf.subarray(0, 6).equals(GIF87) || buf.subarray(0, 6).equals(GIF89))) return { mime: 'image/gif', ext: '.gif' }; - if (buf.length >= 12 - && buf.subarray(0, 4).toString('latin1') === 'RIFF' - && buf.subarray(8, 12).toString('latin1') === 'WEBP') return { mime: 'image/webp', ext: '.webp' }; - return null; -} - -function validateUploadedFile(req: AuthRequest, res: Response, next: Function) { - if (!req.file) return next(); - try { - const fd = fs.openSync(req.file.path, 'r'); - const head = Buffer.alloc(12); - fs.readSync(fd, head, 0, 12, 0); - fs.closeSync(fd); - - const detected = detectType(head); - if (!detected) { - try { fs.unlinkSync(req.file.path); } catch { /* ignore */ } - res.status(415).json({ - success: false, - error: 'Datei-Inhalt entspricht keinem zulässigen Typ (PDF, JPG, PNG, GIF, WebP).', - }); - 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); - const base = path.basename(req.file.path).replace(/\.[^./]+(\.[^./]+)*$/, ''); - const newName = base + detected.ext; - const newPath = path.join(dir, newName); - if (newPath !== req.file.path) { - try { - fs.renameSync(req.file.path, newPath); - req.file.path = newPath; - req.file.filename = newName; - } catch (e) { - // Rename hat seltene Edge-Cases (Cross-Device). Sicherheit - // geht vor – sollte das fehlschlagen, werfen wir lieber 500 - // und putzen die alte Datei. - try { fs.unlinkSync(req.file.path); } catch { /* ignore */ } - console.error('Upload-Rename fehlgeschlagen:', e); - res.status(500).json({ success: false, error: 'Upload konnte nicht abgeschlossen werden' }); - return; - } - } - - // Mimetype mit dem ERKANNTEN überschreiben, damit die Controller - // den korrekten Typ persistieren (falls sie ihn weiterreichen). - req.file.mimetype = detected.mime; - next(); - } catch (e) { - console.error('Magic-Byte-Check fehlgeschlagen:', e); - try { fs.unlinkSync(req.file.path); } catch { /* ignore */ } - res.status(500).json({ success: false, error: 'Upload konnte nicht geprüft werden' }); - } -} +// Magic-Byte-Whitelist + canonical Rename + PDF-Scan: siehe +// middleware/uploadFileTypeValidator.ts (zentralisiert, damit auch +// contract.routes.ts denselben Check fahren kann – Pentest 69.3). // Upload für Bankkarten-Dokumente router.post( diff --git a/docs/todo.md b/docs/todo.md index 5e005cfc..8fcb82da 100644 --- a/docs/todo.md +++ b/docs/todo.md @@ -97,6 +97,25 @@ isolierte Instanz (keine Multi-Tenancy im Code), Provisioning + Abrechnung ## ✅ Erledigt +- [x] **🔒 Pentest 69.3 (INFO → Defense-in-Depth): Magic-Byte-Check auf Vertragsdokumente erweitert** + - `contract.routes.ts` Vertragsdokumente-Upload hatte bisher nur den + PDF-Inhalts-Scan (`scanUploadedPdfIfPresent` aus 68.1). JPG/PNG- + Uploads waren ungeprüft – kompensiert durch Download-Layer + (`fileDownload.controller.ts` liefert nur bei Magic-Byte-Match + inline aus, sonst attachment). Pentester selbst: "ohne Exploit- + Pfad", aber inkonsistent zu `upload.routes.ts`. + - **Refactor:** `detectType` + `validateUploadedFile` aus + `upload.routes.ts` in neue Middleware + `middleware/uploadFileTypeValidator.ts` ausgelagert (Single Source + of Truth). Beide Routes nutzen jetzt denselben Helper. + - **contract.routes.ts:** `validateUploadedFile` ersetzt das + schlankere `scanUploadedPdfIfPresent` – jetzt greift Magic-Byte + + canonical Rename + PDF-Scan für Vertragsdokumente analog zu allen + anderen Upload-Pfaden. + - **pdfUploadSafety.ts:** `scanUploadedPdfIfPresent` entfernt (tot, + da nur in contract.routes verwendet wurde). `requireSafeUploadedPdf` + bleibt für gdpr.routes Vollmacht + pdfTemplate.routes. + - [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