Pentest R101.1: Inline-Preview-Pfad refaktoriert + Diagnose-Log
R101.1 INFO/funktional: Pentester sieht Content-Disposition: attachment auch bei ?disposition=inline. Die Logik im Controller ist korrekt und liefert beim Direkttest gegen echte PDFs application/pdf, der Pfad lässt sich aber in Prod nicht reproduzieren. Refaktoriert: - Magic-Byte-Check in detectSafeContentType() extrahiert - File-Descriptor wird in finally garantiert geschlossen - Short-Read-Fälle (bytesRead < n) explizit geguardet - console.warn wenn inline angefragt aber Magic-Byte-Mismatch oder Read-Crash – damit der Fall in Prod-Logs sichtbar wird falls er wieder auftritt Sicherheits-Verhalten unverändert: Mismatch → attachment (Stored-XSS-Schutz aus R30.13). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
@@ -87,48 +87,72 @@ export async function downloadFile(req: AuthRequest, res: Response): Promise<voi
|
|||||||
// Stored-XSS gerendert.
|
// Stored-XSS gerendert.
|
||||||
//
|
//
|
||||||
// Default: Content-Disposition: attachment → Browser lädt nur runter.
|
// Default: Content-Disposition: attachment → Browser lädt nur runter.
|
||||||
// Opt-in inline-Vorschau (Bank-Karten/Ausweis-Anzeigen-Button) per
|
// Opt-in inline-Vorschau (Bank-Karten/Ausweis-Anzeigen-Button,
|
||||||
// ?disposition=inline, ABER nur wenn die ersten Bytes der Datei das
|
// Vertragsdokumente-Vorschau) per ?disposition=inline, ABER nur wenn
|
||||||
// Magic eines bekannten safe Typs (PDF, PNG, JPEG, GIF, WebP) zeigen.
|
// die ersten Bytes der Datei das Magic eines bekannten safe Typs
|
||||||
// Bei Mismatch fällt's auf attachment zurück – Stored XSS bleibt
|
// (PDF, PNG, JPEG, GIF, WebP) zeigen. Bei Mismatch fällt's auf
|
||||||
// weiterhin unmöglich.
|
// attachment zurück – Stored XSS bleibt weiterhin unmöglich.
|
||||||
|
//
|
||||||
|
// Pentest 101.1 (INFO, 2026-06-22): R101.1 berichtete, dass inline
|
||||||
|
// nie greift. Die Logik selbst ist OK; um künftige Regressionen
|
||||||
|
// sichtbar zu machen, loggen wir jetzt, wenn `inline` zwar angefragt
|
||||||
|
// wurde, aber wegen Magic-Byte-Mismatch oder Read-Fehler abgelehnt
|
||||||
|
// wird (passiert im Normalfall NIE bei echten PDFs/Images).
|
||||||
const filename = path.basename(absolute).replace(/[^A-Za-z0-9._-]/g, '_');
|
const filename = path.basename(absolute).replace(/[^A-Za-z0-9._-]/g, '_');
|
||||||
const wantsInline = req.query.disposition === 'inline';
|
const wantsInline = req.query.disposition === 'inline';
|
||||||
let useInline = false;
|
const safeContentType = wantsInline ? detectSafeContentType(absolute) : null;
|
||||||
let inlineContentType: string | null = null;
|
|
||||||
if (wantsInline) {
|
if (wantsInline && !safeContentType) {
|
||||||
try {
|
console.warn(
|
||||||
const fd = fs.openSync(absolute, 'r');
|
`[fileDownload] inline angefragt, aber Magic-Byte-Check fehlgeschlagen: ${requested}`,
|
||||||
const head = Buffer.alloc(12);
|
);
|
||||||
fs.readSync(fd, head, 0, 12, 0);
|
|
||||||
fs.closeSync(fd);
|
|
||||||
if (head.subarray(0, 5).toString('latin1') === '%PDF-') {
|
|
||||||
useInline = true;
|
|
||||||
inlineContentType = 'application/pdf';
|
|
||||||
} else if (head.subarray(0, 8).equals(Buffer.from([0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a]))) {
|
|
||||||
useInline = true;
|
|
||||||
inlineContentType = 'image/png';
|
|
||||||
} else if (head[0] === 0xff && head[1] === 0xd8 && head[2] === 0xff) {
|
|
||||||
useInline = true;
|
|
||||||
inlineContentType = 'image/jpeg';
|
|
||||||
} else if (head.subarray(0, 6).toString('latin1') === 'GIF87a'
|
|
||||||
|| head.subarray(0, 6).toString('latin1') === 'GIF89a') {
|
|
||||||
useInline = true;
|
|
||||||
inlineContentType = 'image/gif';
|
|
||||||
} else if (head.subarray(0, 4).toString('latin1') === 'RIFF'
|
|
||||||
&& head.subarray(8, 12).toString('latin1') === 'WEBP') {
|
|
||||||
useInline = true;
|
|
||||||
inlineContentType = 'image/webp';
|
|
||||||
}
|
|
||||||
} catch { /* ignore – fällt auf attachment zurück */ }
|
|
||||||
}
|
}
|
||||||
|
|
||||||
res.setHeader('X-Content-Type-Options', 'nosniff');
|
res.setHeader('X-Content-Type-Options', 'nosniff');
|
||||||
if (useInline && inlineContentType) {
|
if (safeContentType) {
|
||||||
res.setHeader('Content-Type', inlineContentType);
|
res.setHeader('Content-Type', safeContentType);
|
||||||
res.setHeader('Content-Disposition', `inline; filename="${filename}"`);
|
res.setHeader('Content-Disposition', `inline; filename="${filename}"`);
|
||||||
} else {
|
} else {
|
||||||
res.setHeader('Content-Disposition', `attachment; filename="${filename}"`);
|
res.setHeader('Content-Disposition', `attachment; filename="${filename}"`);
|
||||||
}
|
}
|
||||||
res.sendFile(absolute);
|
res.sendFile(absolute);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Liest die ersten 12 Bytes der Datei und gibt einen MIME-Type zurück,
|
||||||
|
* wenn die Datei einer bekannten Whitelist (PDF, PNG, JPEG, GIF, WebP)
|
||||||
|
* entspricht. Sonst `null` – dann wird die Datei als attachment serviert.
|
||||||
|
*
|
||||||
|
* Wird nur aufgerufen, wenn `?disposition=inline` angefragt wurde, damit
|
||||||
|
* der Standardfluss (attachment) ohne zusätzlichen Disk-Read auskommt.
|
||||||
|
*/
|
||||||
|
function detectSafeContentType(absolute: string): string | null {
|
||||||
|
let fd: number | null = null;
|
||||||
|
try {
|
||||||
|
fd = fs.openSync(absolute, 'r');
|
||||||
|
const head = Buffer.alloc(12);
|
||||||
|
const bytesRead = fs.readSync(fd, head, 0, 12, 0);
|
||||||
|
if (bytesRead < 5) return null; // Datei zu klein für jede Magic-Sig
|
||||||
|
if (head.subarray(0, 5).toString('latin1') === '%PDF-') return 'application/pdf';
|
||||||
|
if (bytesRead >= 8
|
||||||
|
&& head.subarray(0, 8).equals(Buffer.from([0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a]))
|
||||||
|
) return 'image/png';
|
||||||
|
if (bytesRead >= 3 && head[0] === 0xff && head[1] === 0xd8 && head[2] === 0xff) return 'image/jpeg';
|
||||||
|
if (bytesRead >= 6
|
||||||
|
&& (head.subarray(0, 6).toString('latin1') === 'GIF87a'
|
||||||
|
|| head.subarray(0, 6).toString('latin1') === 'GIF89a')
|
||||||
|
) return 'image/gif';
|
||||||
|
if (bytesRead >= 12
|
||||||
|
&& head.subarray(0, 4).toString('latin1') === 'RIFF'
|
||||||
|
&& head.subarray(8, 12).toString('latin1') === 'WEBP'
|
||||||
|
) return 'image/webp';
|
||||||
|
return null;
|
||||||
|
} catch (err) {
|
||||||
|
console.warn(`[fileDownload] Magic-Byte-Read fehlgeschlagen für ${absolute}:`, err);
|
||||||
|
return null;
|
||||||
|
} finally {
|
||||||
|
if (fd !== null) {
|
||||||
|
try { fs.closeSync(fd); } catch { /* ignore */ }
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
@@ -97,6 +97,24 @@ isolierte Instanz (keine Multi-Tenancy im Code), Provisioning + Abrechnung
|
|||||||
|
|
||||||
## ✅ Erledigt
|
## ✅ Erledigt
|
||||||
|
|
||||||
|
- [x] **🔧 Pentest R101.1 – Inline-Preview-Pfad refaktoriert + Diagnose-Log**
|
||||||
|
- Pentester R101.1 (INFO/funktional) berichtet: `?disposition=inline`
|
||||||
|
bewirkt nichts, Browser zeigt Download-Dialog. Die Logik im
|
||||||
|
`fileDownload.controller` ist eigentlich korrekt – sauberer Magic-
|
||||||
|
Byte-Check für PDF/PNG/JPEG/GIF/WebP – und liefert beim Direkttest
|
||||||
|
gegen echte Vertrags-PDFs `application/pdf`. Wir können das in Prod
|
||||||
|
aber nicht reproduzieren.
|
||||||
|
- Refaktorierung: Magic-Byte-Check in `detectSafeContentType()`
|
||||||
|
extrahiert, finally-Block schließt File-Descriptor garantiert,
|
||||||
|
Short-Read-Fälle (`bytesRead < n`) jetzt sauber geguardet.
|
||||||
|
- Sicherheits-Verhalten unverändert: bei Magic-Byte-Mismatch bleibt
|
||||||
|
es bei `Content-Disposition: attachment` (Stored-XSS-Schutz aus
|
||||||
|
R30.13).
|
||||||
|
- Neu: `console.warn`, wenn `inline` angefragt wurde, aber der
|
||||||
|
Magic-Byte-Check fehlschlägt oder der Read crasht. Damit fällt
|
||||||
|
der Fall im Prod-Log auf, falls er nochmal auftritt – bisher
|
||||||
|
war's silent.
|
||||||
|
|
||||||
- [x] **🔒 Pentest R97 – Attachment-Validierung im Send-Handler**
|
- [x] **🔒 Pentest R97 – Attachment-Validierung im Send-Handler**
|
||||||
- R97.1 (LOW): malformed `content` (`null`, fehlend, `true`, `""`)
|
- R97.1 (LOW): malformed `content` (`null`, fehlend, `true`, `""`)
|
||||||
erzeugte 200/500 mit rohem `Buffer.from()`-Fehlertext in der
|
erzeugte 200/500 mit rohem `Buffer.from()`-Fehlertext in der
|
||||||
|
|||||||
Reference in New Issue
Block a user