From 87ac5fa1a30f622c7b99715b8aeafc774f160a6a Mon Sep 17 00:00:00 2001 From: Bogdan Date: Mon, 7 Sep 2026 22:00:49 +0200 Subject: [PATCH 1/2] fix(HTTP): strip CRLF from DownloadResponse filename to prevent header injection Sanitize carriage return and newline characters from filename in DownloadResponse to prevent HTTP response splitting and invalid header values per RFC 7230. --- system/HTTP/DownloadResponse.php | 8 +++---- tests/system/HTTP/DownloadResponseTest.php | 24 +++++++++++++++++++++ user_guide_src/source/changelogs/v4.7.5.rst | 1 + 3 files changed, 29 insertions(+), 4 deletions(-) diff --git a/system/HTTP/DownloadResponse.php b/system/HTTP/DownloadResponse.php index b7ce79b8309a..a79f50d5eef5 100644 --- a/system/HTTP/DownloadResponse.php +++ b/system/HTTP/DownloadResponse.php @@ -71,7 +71,7 @@ public function __construct(string $filename, bool $setMime) { parent::__construct(config(App::class)); - $this->filename = $filename; + $this->filename = str_replace(["\r", "\n"], '', $filename); $this->setMime = $setMime; // Make sure the content type is either specified or detected @@ -113,7 +113,7 @@ public function setFilePath(string $filepath) */ public function setFileName(string $filename) { - $this->filename = $filename; + $this->filename = str_replace(["\r", "\n"], '', $filename); return $this; } @@ -161,8 +161,8 @@ private function setContentTypeByMimeType(): void */ private function getDownloadFileName(): string { - $filename = $this->filename; - $x = explode('.', $this->filename); + $filename = $this->filename; + $x = explode('.', $filename); $extension = end($x); /* It was reported that browsers on Android 2.1 (and possibly older as well) diff --git a/tests/system/HTTP/DownloadResponseTest.php b/tests/system/HTTP/DownloadResponseTest.php index d5fea35e380c..b0f34f2c1f4f 100644 --- a/tests/system/HTTP/DownloadResponseTest.php +++ b/tests/system/HTTP/DownloadResponseTest.php @@ -154,6 +154,30 @@ public function testDispositionInlineWithSetFileName(): void $this->assertSame('inline; filename="my\"quoted\"File.txt"; filename*=UTF-8\'\'my%22quoted%22File.txt', $response->getHeaderLine('Content-Disposition')); } + public function testContentDispositionSanitizesCRLF(): void + { + $response = new DownloadResponse("test\r\nfile.txt", false); + $response->buildHeaders(); + + $header = $response->getHeaderLine('Content-Disposition'); + $this->assertStringNotContainsString("\r", $header); + $this->assertStringNotContainsString("\n", $header); + $this->assertSame('attachment; filename="testfile.txt"; filename*=UTF-8\'\'testfile.txt', $header); + + $response = new DownloadResponse('report.pdf', false); + $response->setFileName("report\r\nSet-Cookie: evil=1\r\n.pdf"); + + // The filename must be sanitized to strip carriage returns and newlines, + // preventing HTTP response splitting / header injection without throwing an RFC 7230 error. + $response->buildHeaders(); + + $header = $response->getHeaderLine('Content-Disposition'); + $this->assertStringNotContainsString("\r", $header); + $this->assertStringNotContainsString("\n", $header); + $this->assertFalse($response->hasHeader('Set-Cookie')); + $this->assertSame('attachment; filename="reportSet-Cookie: evil=1.pdf"; filename*=UTF-8\'\'reportSet-Cookie%3A%20evil%3D1.pdf', $header); + } + public function testNoCache(): void { $response = new DownloadResponse('unit-test.txt', true); diff --git a/user_guide_src/source/changelogs/v4.7.5.rst b/user_guide_src/source/changelogs/v4.7.5.rst index f13a3e4b2b49..db1bc68d3875 100644 --- a/user_guide_src/source/changelogs/v4.7.5.rst +++ b/user_guide_src/source/changelogs/v4.7.5.rst @@ -43,6 +43,7 @@ Bugs Fixed - **Content Security Policy:** Fixed a bug where empty ``Content-Security-Policy``, ``Content-Security-Policy-Report-Only``, and ``Reporting-Endpoints`` response headers were generated when no corresponding values existed. - **Cookie:** Fixed a bug where ``Cookie`` instances created with ``raw: true`` allowed invalid characters in cookie values rejected by ``setrawcookie()``. - **Database:** Fixed a bug where rebuilding a SQLite3 table (e.g., ``Forge::dropColumn()``, ``Forge::modifyColumn()``, ``Forge::dropForeignKey()`` and ``Forge::dropPrimaryKey()``) corrupted the table names referenced by its foreign keys when ``DBPrefix`` was set. +- **DownloadResponse:** Fixed a bug where filenames containing carriage returns or newlines were not sanitized, potentially leading to HTTP response splitting (CRLF / header injection). - **Files:** Fixed a bug where ``File::move()`` and ``UploadedFile::move()`` set executable and overly permissive file permissions (``0777 & ~umask()`` instead of ``0666 & ~umask()``), and ``UploadedFile::move()`` targeted the parent directory instead of the destination file for ``chmod()``. - **Helpers:** Fixed a bug where ``get_dir_file_info()`` returned incomplete entries for subdirectories and missing files instead of omitting them. - **Honeypot:** Fixed a bug where bot detection returned an HTTP 500 response instead of 403 (Forbidden). From 4bd266546435516576ba5a63835393476f87ffa1 Mon Sep 17 00:00:00 2001 From: Bogdan Date: Wed, 9 Sep 2026 20:48:31 +0200 Subject: [PATCH 2/2] style: fix alignment of variable assignments in getDownloadFileName --- system/HTTP/DownloadResponse.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/system/HTTP/DownloadResponse.php b/system/HTTP/DownloadResponse.php index a79f50d5eef5..ef0c5d94f430 100644 --- a/system/HTTP/DownloadResponse.php +++ b/system/HTTP/DownloadResponse.php @@ -161,8 +161,8 @@ private function setContentTypeByMimeType(): void */ private function getDownloadFileName(): string { - $filename = $this->filename; - $x = explode('.', $filename); + $filename = $this->filename; + $x = explode('.', $filename); $extension = end($x); /* It was reported that browsers on Android 2.1 (and possibly older as well)