From 3e5810990cf17be166ca4ccfc9d083a3688d60f7 Mon Sep 17 00:00:00 2001 From: fds2610-bot Date: Tue, 6 Oct 2026 15:20:21 +0200 Subject: [PATCH] Fix #217: Content-Disposition-Header gegen Parameter-Injection maskieren Co-Authored-By: Claude Sonnet 5 --- Changes.md | 7 +++ src/Controllers/BelegController.php | 4 +- src/Controllers/InvoiceController.php | 2 +- src/View.php | 14 ++++++ tests/103_ContentDispositionHeaderTest.php | 55 ++++++++++++++++++++++ tests/67_BelegXmlXsltXssTest.php | 2 +- 6 files changed, 80 insertions(+), 4 deletions(-) create mode 100644 tests/103_ContentDispositionHeaderTest.php diff --git a/Changes.md b/Changes.md index 4a3181f..86929db 100644 --- a/Changes.md +++ b/Changes.md @@ -2,6 +2,13 @@ --- +## 2026-10-06 – Header-Injection über Dateinamen im Content-Disposition-Header behoben (#217) + +- **`src/View.php`:** neue Methode `View::contentDispositionHeader(string $disposition, string $filename): string`. `PATHINFO_BASENAME` entfernt beim Beleg-Upload nur Verzeichnisanteile, nicht aber Anführungszeichen oder Backslashes im client-seitig gewählten Dateinamen – ein präparierter Name wie `legit"; filename*=UTF-8''%2e%2e%2fevil.exe` brach beim Download aus dem `filename="..."`-Parameter aus und schleuste einen zusätzlichen, von vielen Browsern bevorzugt ausgewerteten `filename*=`-Parameter ein. Damit ließ sich der im Download-Dialog angezeigte Dateiname/die Endung gegenüber anderen Nutzern fälschen (Dateityp-Spoofing). Die neue Methode maskiert Anführungszeichen, Backslashes und Steuerzeichen im ASCII-`filename`-Parameter und setzt zusätzlich RFC-6266-konform `filename*=UTF-8''...` mit dem unveränderten, `rawurlencode()`-kodierten Namen. +- **`src/Controllers/BelegController.php`** (`serveFile()`, zwei Stellen) **und `src/Controllers/InvoiceController.php`** (`pdf()`): nutzen jetzt `View::contentDispositionHeader()` statt den Dateinamen direkt in den Header zu interpolieren. +- **`tests/103_ContentDispositionHeaderTest.php`** (neu): prüft, dass ein eingeschleuster `filename*=`-Parameter nicht aus dem Header ausbricht, dass kein CR/LF durchschlägt und dass unauffällige Dateinamen unverändert bleiben. +- **`tests/67_BelegXmlXsltXssTest.php`:** Quelltext-Prüfung an die neue Helper-Methode angepasst (prüft weiterhin, dass `serveFile()` die ermittelte Disposition statt eines festen `inline` verwendet). + ## 2026-09-29 – Sessions deaktivierter/gelöschter Benutzer sofort ungültig (#215) - **`src/Auth.php`:** `Auth::require()` lud den Benutzer bislang nie neu aus der DB – nur `$_SESSION['user_id']` und das 24h-Alter der Session zählten. Deaktivierte, gelöschte oder im Passwort geänderte Konten behielten dadurch bis zu 24h vollen Zugriff, auch auf Admin-Funktionen. `require()` lädt jetzt bei jedem Request `User::findAuthData()` nach und verwirft die Session (Logout + Redirect `/login`), sobald der Benutzer fehlt, `active = 0` ist oder ein in der Session hinterlegter Passwort-Fingerprint nicht mehr zum aktuellen `password_hash` passt. Die Rolle wird dabei jedes Mal frisch aus der DB in die Session übernommen, damit auch eine Rollenänderung sofort greift statt erst beim nächsten Login. Der Fingerprint (`Auth::passwordFingerprint()`, SHA-256 über den bcrypt-Hash) wird beim Login in `$_SESSION['pwd_fp']` abgelegt – Bestandssessions ohne diesen Wert werden beim nächsten Request einmalig abgemeldet. diff --git a/src/Controllers/BelegController.php b/src/Controllers/BelegController.php index 69bf435..af83a3c 100644 --- a/src/Controllers/BelegController.php +++ b/src/Controllers/BelegController.php @@ -345,7 +345,7 @@ class BelegController extends BaseController $pdf = $svc->renderToString($invoice, $lines, $settings); header('Content-Type: application/pdf'); - header('Content-Disposition: inline; filename="' . $beleg['filename'] . '"'); + header('Content-Disposition: ' . View::contentDispositionHeader('inline', $beleg['filename'])); header('Content-Length: ' . strlen($pdf)); echo $pdf; exit; @@ -367,7 +367,7 @@ class BelegController extends BaseController $disposition = $isXml ? 'attachment' : 'inline'; header('Content-Type: ' . $beleg['mime_type']); - header('Content-Disposition: ' . $disposition . '; filename="' . $beleg['filename'] . '"'); + header('Content-Disposition: ' . View::contentDispositionHeader($disposition, $beleg['filename'])); header('Content-Length: ' . filesize($realPath)); readfile($realPath); } diff --git a/src/Controllers/InvoiceController.php b/src/Controllers/InvoiceController.php index 1005149..3c9e468 100644 --- a/src/Controllers/InvoiceController.php +++ b/src/Controllers/InvoiceController.php @@ -285,7 +285,7 @@ class InvoiceController extends BaseController : ''; if ($absPath && is_file($absPath)) { header('Content-Type: application/pdf'); - header('Content-Disposition: attachment; filename="' . $beleg['filename'] . '"'); + header('Content-Disposition: ' . View::contentDispositionHeader('attachment', $beleg['filename'])); header('Content-Length: ' . filesize($absPath)); readfile($absPath); return; diff --git a/src/View.php b/src/View.php index 8a06a94..49c5be0 100644 --- a/src/View.php +++ b/src/View.php @@ -90,6 +90,20 @@ class View return $d ? $d->format('d.m.Y') : $date; } + /** + * Baut den Wert des Content-Disposition-Headers für einen (ggf. vom Nutzer + * beim Upload frei wählbaren) Dateinamen. PATHINFO_BASENAME entfernt zwar + * Verzeichnisanteile, nicht aber Anführungszeichen/Backslashes – ohne + * Maskierung könnte ein präparierter Dateiname aus dem filename="..."-Parameter + * ausbrechen und einen zusätzlichen filename*=-Parameter einschleusen, der von + * vielen Browsern bevorzugt ausgewertet wird (Issue #217). + */ + public static function contentDispositionHeader(string $disposition, string $filename): string + { + $safe = (string)preg_replace('/[\x00-\x1f"\\\\]/', '_', $filename); + return $disposition . '; filename="' . $safe . '"; filename*=UTF-8\'\'' . rawurlencode($filename); + } + public static function buildQuery(array $params): string { $safe = []; diff --git a/tests/103_ContentDispositionHeaderTest.php b/tests/103_ContentDispositionHeaderTest.php new file mode 100644 index 0000000..e33b07b --- /dev/null +++ b/tests/103_ContentDispositionHeaderTest.php @@ -0,0 +1,55 @@ +assert( + !str_contains($header, 'legit"; filename*='), + 'Eingeschleustes filename*= bricht nicht aus dem ersten Parameter aus' + ); + $this->assertEquals( + 1, + substr_count($header, '"; filename*='), + 'Es gibt nur ein echtes filename*=-Parameter (direkt nach dem schließenden Anführungszeichen)' + ); + $this->assert(!str_contains($header, "\r") && !str_contains($header, "\n"), 'Header enthält kein CR/LF'); + + $mitAnführungszeichen = View::contentDispositionHeader('attachment', 'a"b\\c.pdf'); + $this->assert( + !preg_match('/filename="[^"]*"[^"]*"/', $mitAnführungszeichen), + 'Anführungszeichen im Dateinamen schließen den filename-Parameter nicht vorzeitig' + ); + + $umlaut = View::contentDispositionHeader('inline', 'Rechnung Übersicht äöü.pdf'); + $this->assert( + str_contains($umlaut, "filename*=UTF-8''" . rawurlencode('Rechnung Übersicht äöü.pdf')), + 'filename* transportiert den unveränderten UTF-8-Namen RFC-6266-konform' + ); + + $normal = View::contentDispositionHeader('attachment', 'Rechnung-2026-001.pdf'); + $this->assertEquals( + 'attachment; filename="Rechnung-2026-001.pdf"; filename*=UTF-8\'\'Rechnung-2026-001.pdf', + $normal, + 'Unauffällige Dateinamen bleiben unverändert' + ); + } +} diff --git a/tests/67_BelegXmlXsltXssTest.php b/tests/67_BelegXmlXsltXssTest.php index c63141c..82aaa1a 100644 --- a/tests/67_BelegXmlXsltXssTest.php +++ b/tests/67_BelegXmlXsltXssTest.php @@ -82,7 +82,7 @@ class BelegXmlXsltXssTest extends TestCase 'XML wird als attachment ausgeliefert, alles andere weiterhin inline' ); $this->assert( - (bool)preg_match("/Content-Disposition:\s*'\s*\.\s*\\\$disposition/", $serveFile), + (bool)preg_match("/contentDispositionHeader\(\s*\\\$disposition\s*,/", $serveFile), 'Der Content-Disposition-Header nutzt die ermittelte Disposition (nicht fest inline)' ); }