Pentest 69.3 (INFO): Magic-Byte-Validator auf Vertragsdokumente erweitert
contract.routes.ts Vertragsdokumente-Upload hatte bisher nur den PDF-Inhalts-Scan aus 68.1. JPG/PNG-Uploads waren ungeprüft, ohne canonical Rename – Pentester selbst attestiert "ohne Exploit-Pfad" (Download-Layer fängt's), aber inkonsistent zu allen anderen Upload-Pfaden. - Refactor: detectType + validateUploadedFile aus upload.routes.ts in neue Middleware uploadFileTypeValidator.ts ausgelagert (Single Source of Truth, ~90 Zeilen Duplikation entfällt). - contract.routes.ts: validateUploadedFile ersetzt scanUploadedPdfIfPresent → Magic-Byte + canonical Rename + PDF-Scan in einer Pipeline. - pdfUploadSafety.ts: scanUploadedPdfIfPresent entfernt (tot).
This commit is contained in:
@@ -3,42 +3,14 @@ import fs from 'fs';
|
|||||||
import { assertSafePdf } from '../utils/sanitize.js';
|
import { assertSafePdf } from '../utils/sanitize.js';
|
||||||
import { ApiError } from '../utils/apiError.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
|
* 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 {
|
export function requireSafeUploadedPdf(req: Request, res: Response, next: NextFunction): void {
|
||||||
const file = (req as Request & { file?: Express.Multer.File }).file;
|
const file = (req as Request & { file?: Express.Multer.File }).file;
|
||||||
|
|||||||
@@ -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' });
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -5,7 +5,7 @@ import fs from 'fs';
|
|||||||
import * as contractController from '../controllers/contract.controller.js';
|
import * as contractController from '../controllers/contract.controller.js';
|
||||||
import * as invoiceController from '../controllers/invoice.controller.js';
|
import * as invoiceController from '../controllers/invoice.controller.js';
|
||||||
import { authenticate, requirePermission } from '../middleware/auth.js';
|
import { authenticate, requirePermission } from '../middleware/auth.js';
|
||||||
import { scanUploadedPdfIfPresent } from '../middleware/pdfUploadSafety.js';
|
import { validateUploadedFile } from '../middleware/uploadFileTypeValidator.js';
|
||||||
|
|
||||||
const router = Router();
|
const router = Router();
|
||||||
|
|
||||||
@@ -55,7 +55,7 @@ router.post('/:id/invoices', authenticate, requirePermission('contracts:update')
|
|||||||
|
|
||||||
// Vertragsdokumente
|
// Vertragsdokumente
|
||||||
router.get('/:id/documents', authenticate, requirePermission('contracts:read'), contractController.getContractDocuments);
|
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);
|
router.delete('/:id/documents/:documentId', authenticate, requirePermission('contracts:update'), contractController.deleteContractDocument);
|
||||||
|
|
||||||
// Folgezähler
|
// Folgezähler
|
||||||
|
|||||||
@@ -12,8 +12,8 @@ import {
|
|||||||
canAccessBankCard,
|
canAccessBankCard,
|
||||||
canAccessIdentityDocument,
|
canAccessIdentityDocument,
|
||||||
} from '../utils/accessControl.js';
|
} from '../utils/accessControl.js';
|
||||||
import { validateOptionalIsoDate, assertSafePdf } from '../utils/sanitize.js';
|
import { validateOptionalIsoDate } from '../utils/sanitize.js';
|
||||||
import { ApiError } from '../utils/apiError.js';
|
import { validateUploadedFile } from '../middleware/uploadFileTypeValidator.js';
|
||||||
|
|
||||||
// Pentest 56.1 (HIGH, 2026-06-01): Upload-Endpoints prüften nur die
|
// Pentest 56.1 (HIGH, 2026-06-01): Upload-Endpoints prüften nur die
|
||||||
// Permission, nicht ob die Ziel-Resource zum Caller passt. Helper-Funktion
|
// Permission, nicht ob die Ziel-Resource zum Caller passt. Helper-Funktion
|
||||||
@@ -95,98 +95,9 @@ function setUploadDir(subDir: string) {
|
|||||||
};
|
};
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
// Magic-Byte-Whitelist + canonical Rename + PDF-Scan: siehe
|
||||||
* Post-Upload-Validierung: prüft die Magic-Bytes der gerade geschriebenen
|
// middleware/uploadFileTypeValidator.ts (zentralisiert, damit auch
|
||||||
* Datei und vergleicht mit dem fileFilter-Whitelist. Bei Mismatch
|
// contract.routes.ts denselben Check fahren kann – Pentest 69.3).
|
||||||
* (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
|
|
||||||
* `<timestamp-random>.<canonical-ext>` (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
|
|
||||||
// `<unique>.gif.php` o.ä. geschrieben – wir wollen `<unique>.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' });
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
// Upload für Bankkarten-Dokumente
|
// Upload für Bankkarten-Dokumente
|
||||||
router.post(
|
router.post(
|
||||||
|
|||||||
@@ -97,6 +97,25 @@ isolierte Instanz (keine Multi-Tenancy im Code), Provisioning + Abrechnung
|
|||||||
|
|
||||||
## ✅ Erledigt
|
## ✅ 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**
|
- [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
|
- **68.1 PDF-Active-Content-Filter:** Magic-Byte-Check prüfte bisher
|
||||||
nur `%PDF-`. PDFs mit `/JavaScript`, `/JS`, `/Launch` (externes
|
nur `%PDF-`. PDFs mit `/JavaScript`, `/JS`, `/Launch` (externes
|
||||||
|
|||||||
Reference in New Issue
Block a user