Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions system/HTTP/DownloadResponse.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -161,8 +161,8 @@ private function setContentTypeByMimeType(): void
*/
private function getDownloadFileName(): string
{
$filename = $this->filename;
$x = explode('.', $this->filename);
$filename = str_replace(["\r", "\n"], '', $this->filename);
$x = explode('.', $filename);
$extension = end($x);

/* It was reported that browsers on Android 2.1 (and possibly older as well)
Expand Down
24 changes: 24 additions & 0 deletions tests/system/HTTP/DownloadResponseTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
1 change: 1 addition & 0 deletions user_guide_src/source/changelogs/v4.7.5.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand Down
Loading