Merge pull request 'Fix #217: Content-Disposition-Header gegen Parameter-Injection maskieren' (#218) from fds2610-bot/Steuerkram:main into main

Reviewed-on: https://codeberg.org/fds2610/Steuerkram/pulls/218

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.

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);
     }

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;

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 = [];

tests/103_ContentDispositionHeaderTest.php

@@ -0,0 +1,55 @@
+<?php
+
+declare(strict_types=1);
+
+use Steuerkram\View;
+
+/**
+ * Test 103: Content-Disposition-Header ohne Parameter-Injection über den
+ * Dateinamen (Issue #217)
+ *
+ * Beim Beleg-Upload übernimmt die Anwendung den vom Client gesendeten
+ * Dateinamen unverändert. View::contentDispositionHeader() muss verhindern,
+ * dass ein präparierter Dateiname aus dem filename="..."-Parameter ausbricht
+ * und einen zusätzlichen filename*=-Parameter einschleust.
+ */
+class ContentDispositionHeaderTest extends TestCase
+{
+    protected string $testName = 'Test 103: Content-Disposition ohne Header-Injection';
+
+    public function run(): void
+    {
+        $bösartig = 'legit"; filename*=UTF-8\'\'%2e%2e%2fevil.exe';
+        $header   = View::contentDispositionHeader('inline', $bösartig);
+
+        $this->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'
+        );
+    }
+}

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)'
         );
     }