Fix #217: Content-Disposition-Header gegen Parameter-Injection maskieren
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
parent
a53c4c3dc8
commit
3e5810990c
6 changed files with 80 additions and 4 deletions
|
|
@ -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)
|
## 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/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.
|
||||||
|
|
|
||||||
|
|
@ -345,7 +345,7 @@ class BelegController extends BaseController
|
||||||
$pdf = $svc->renderToString($invoice, $lines, $settings);
|
$pdf = $svc->renderToString($invoice, $lines, $settings);
|
||||||
|
|
||||||
header('Content-Type: application/pdf');
|
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));
|
header('Content-Length: ' . strlen($pdf));
|
||||||
echo $pdf;
|
echo $pdf;
|
||||||
exit;
|
exit;
|
||||||
|
|
@ -367,7 +367,7 @@ class BelegController extends BaseController
|
||||||
$disposition = $isXml ? 'attachment' : 'inline';
|
$disposition = $isXml ? 'attachment' : 'inline';
|
||||||
|
|
||||||
header('Content-Type: ' . $beleg['mime_type']);
|
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));
|
header('Content-Length: ' . filesize($realPath));
|
||||||
readfile($realPath);
|
readfile($realPath);
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -285,7 +285,7 @@ class InvoiceController extends BaseController
|
||||||
: '';
|
: '';
|
||||||
if ($absPath && is_file($absPath)) {
|
if ($absPath && is_file($absPath)) {
|
||||||
header('Content-Type: application/pdf');
|
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));
|
header('Content-Length: ' . filesize($absPath));
|
||||||
readfile($absPath);
|
readfile($absPath);
|
||||||
return;
|
return;
|
||||||
|
|
|
||||||
14
src/View.php
14
src/View.php
|
|
@ -90,6 +90,20 @@ class View
|
||||||
return $d ? $d->format('d.m.Y') : $date;
|
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
|
public static function buildQuery(array $params): string
|
||||||
{
|
{
|
||||||
$safe = [];
|
$safe = [];
|
||||||
|
|
|
||||||
55
tests/103_ContentDispositionHeaderTest.php
Normal file
55
tests/103_ContentDispositionHeaderTest.php
Normal file
|
|
@ -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'
|
||||||
|
);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
@ -82,7 +82,7 @@ class BelegXmlXsltXssTest extends TestCase
|
||||||
'XML wird als attachment ausgeliefert, alles andere weiterhin inline'
|
'XML wird als attachment ausgeliefert, alles andere weiterhin inline'
|
||||||
);
|
);
|
||||||
$this->assert(
|
$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)'
|
'Der Content-Disposition-Header nutzt die ermittelte Disposition (nicht fest inline)'
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue