From a9ffd3e0c77019916bba21830d8514cca723ce17 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Mon, 16 Mar 2026 18:28:44 +0000 Subject: [PATCH] Responses: Added extra sanitization for download names From testing, don't think this could exploited directly, as the response would error instead of allowing control characters, but this adds an extra layer of sanitization, and switches to encoded disposition filenames for better UTF8 support. --- app/Http/DownloadResponseFactory.php | 7 +++++-- tests/Api/ExportsApiTest.php | 30 ++++++++++++++-------------- tests/Exports/HtmlExportTest.php | 6 +++--- tests/Exports/MarkdownExportTest.php | 2 +- tests/Exports/PdfExportTest.php | 6 +++--- tests/Exports/TextExportTest.php | 6 +++--- tests/Uploads/AttachmentTest.php | 19 ++++++++++++++++-- 7 files changed, 47 insertions(+), 29 deletions(-) diff --git a/app/Http/DownloadResponseFactory.php b/app/Http/DownloadResponseFactory.php index 8384484ad..1b6256c00 100644 --- a/app/Http/DownloadResponseFactory.php +++ b/app/Http/DownloadResponseFactory.php @@ -102,12 +102,15 @@ class DownloadResponseFactory protected function getHeaders(string $fileName, int $fileSize, string $mime = 'application/octet-stream'): array { $disposition = ($mime === 'application/octet-stream') ? 'attachment' : 'inline'; - $downloadName = str_replace('"', '', $fileName); + + $downloadName = str_replace(['"', '/', '\\', '$'], '', $fileName); + $downloadName = preg_replace('/[\x00-\x1F\x7F]/', '', $downloadName); + $encodedDownloadName = rawurlencode($downloadName); return [ 'Content-Type' => $mime, 'Content-Length' => $fileSize, - 'Content-Disposition' => "{$disposition}; filename=\"{$downloadName}\"", + 'Content-Disposition' => "{$disposition}; filename*=UTF-8''{$encodedDownloadName}", 'X-Content-Type-Options' => 'nosniff', ]; } diff --git a/tests/Api/ExportsApiTest.php b/tests/Api/ExportsApiTest.php index e1ac698d0..7951e04d2 100644 --- a/tests/Api/ExportsApiTest.php +++ b/tests/Api/ExportsApiTest.php @@ -19,7 +19,7 @@ class ExportsApiTest extends TestCase $resp = $this->get("/api/books/{$book->id}/export/html"); $resp->assertStatus(200); $resp->assertSee($book->name); - $resp->assertHeader('Content-Disposition', 'attachment; filename="' . $book->slug . '.html"'); + $resp->assertHeader('Content-Disposition', 'attachment; filename*=UTF-8\'\'' . $book->slug . '.html'); } public function test_book_plain_text_endpoint() @@ -30,7 +30,7 @@ class ExportsApiTest extends TestCase $resp = $this->get("/api/books/{$book->id}/export/plaintext"); $resp->assertStatus(200); $resp->assertSee($book->name); - $resp->assertHeader('Content-Disposition', 'attachment; filename="' . $book->slug . '.txt"'); + $resp->assertHeader('Content-Disposition', 'attachment; filename*=UTF-8\'\'' . $book->slug . '.txt'); } public function test_book_pdf_endpoint() @@ -40,7 +40,7 @@ class ExportsApiTest extends TestCase $resp = $this->get("/api/books/{$book->id}/export/pdf"); $resp->assertStatus(200); - $resp->assertHeader('Content-Disposition', 'attachment; filename="' . $book->slug . '.pdf"'); + $resp->assertHeader('Content-Disposition', 'attachment; filename*=UTF-8\'\'' . $book->slug . '.pdf'); } public function test_book_markdown_endpoint() @@ -50,7 +50,7 @@ class ExportsApiTest extends TestCase $resp = $this->get("/api/books/{$book->id}/export/markdown"); $resp->assertStatus(200); - $resp->assertHeader('Content-Disposition', 'attachment; filename="' . $book->slug . '.md"'); + $resp->assertHeader('Content-Disposition', 'attachment; filename*=UTF-8\'\'' . $book->slug . '.md'); $resp->assertSee('# ' . $book->name); $resp->assertSee('# ' . $book->pages()->first()->name); $resp->assertSee('# ' . $book->chapters()->first()->name); @@ -63,7 +63,7 @@ class ExportsApiTest extends TestCase $resp = $this->get("/api/books/{$book->id}/export/zip"); $resp->assertStatus(200); - $resp->assertHeader('Content-Disposition', 'attachment; filename="' . $book->slug . '.zip"'); + $resp->assertHeader('Content-Disposition', 'attachment; filename*=UTF-8\'\'' . $book->slug . '.zip'); $zip = ZipTestHelper::extractFromZipResponse($resp); $this->assertArrayHasKey('book', $zip->data); @@ -77,7 +77,7 @@ class ExportsApiTest extends TestCase $resp = $this->get("/api/chapters/{$chapter->id}/export/html"); $resp->assertStatus(200); $resp->assertSee($chapter->name); - $resp->assertHeader('Content-Disposition', 'attachment; filename="' . $chapter->slug . '.html"'); + $resp->assertHeader('Content-Disposition', 'attachment; filename*=UTF-8\'\'' . $chapter->slug . '.html'); } public function test_chapter_plain_text_endpoint() @@ -88,7 +88,7 @@ class ExportsApiTest extends TestCase $resp = $this->get("/api/chapters/{$chapter->id}/export/plaintext"); $resp->assertStatus(200); $resp->assertSee($chapter->name); - $resp->assertHeader('Content-Disposition', 'attachment; filename="' . $chapter->slug . '.txt"'); + $resp->assertHeader('Content-Disposition', 'attachment; filename*=UTF-8\'\'' . $chapter->slug . '.txt'); } public function test_chapter_pdf_endpoint() @@ -98,7 +98,7 @@ class ExportsApiTest extends TestCase $resp = $this->get("/api/chapters/{$chapter->id}/export/pdf"); $resp->assertStatus(200); - $resp->assertHeader('Content-Disposition', 'attachment; filename="' . $chapter->slug . '.pdf"'); + $resp->assertHeader('Content-Disposition', 'attachment; filename*=UTF-8\'\'' . $chapter->slug . '.pdf'); } public function test_chapter_markdown_endpoint() @@ -108,7 +108,7 @@ class ExportsApiTest extends TestCase $resp = $this->get("/api/chapters/{$chapter->id}/export/markdown"); $resp->assertStatus(200); - $resp->assertHeader('Content-Disposition', 'attachment; filename="' . $chapter->slug . '.md"'); + $resp->assertHeader('Content-Disposition', 'attachment; filename*=UTF-8\'\'' . $chapter->slug . '.md'); $resp->assertSee('# ' . $chapter->name); $resp->assertSee('# ' . $chapter->pages()->first()->name); } @@ -120,7 +120,7 @@ class ExportsApiTest extends TestCase $resp = $this->get("/api/chapters/{$chapter->id}/export/zip"); $resp->assertStatus(200); - $resp->assertHeader('Content-Disposition', 'attachment; filename="' . $chapter->slug . '.zip"'); + $resp->assertHeader('Content-Disposition', 'attachment; filename*=UTF-8\'\'' . $chapter->slug . '.zip'); $zip = ZipTestHelper::extractFromZipResponse($resp); $this->assertArrayHasKey('chapter', $zip->data); @@ -134,7 +134,7 @@ class ExportsApiTest extends TestCase $resp = $this->get("/api/pages/{$page->id}/export/html"); $resp->assertStatus(200); $resp->assertSee($page->name); - $resp->assertHeader('Content-Disposition', 'attachment; filename="' . $page->slug . '.html"'); + $resp->assertHeader('Content-Disposition', 'attachment; filename*=UTF-8\'\'' . $page->slug . '.html'); } public function test_page_plain_text_endpoint() @@ -145,7 +145,7 @@ class ExportsApiTest extends TestCase $resp = $this->get("/api/pages/{$page->id}/export/plaintext"); $resp->assertStatus(200); $resp->assertSee($page->name); - $resp->assertHeader('Content-Disposition', 'attachment; filename="' . $page->slug . '.txt"'); + $resp->assertHeader('Content-Disposition', 'attachment; filename*=UTF-8\'\'' . $page->slug . '.txt'); } public function test_page_pdf_endpoint() @@ -155,7 +155,7 @@ class ExportsApiTest extends TestCase $resp = $this->get("/api/pages/{$page->id}/export/pdf"); $resp->assertStatus(200); - $resp->assertHeader('Content-Disposition', 'attachment; filename="' . $page->slug . '.pdf"'); + $resp->assertHeader('Content-Disposition', 'attachment; filename*=UTF-8\'\'' . $page->slug . '.pdf'); } public function test_page_markdown_endpoint() @@ -166,7 +166,7 @@ class ExportsApiTest extends TestCase $resp = $this->get("/api/pages/{$page->id}/export/markdown"); $resp->assertStatus(200); $resp->assertSee('# ' . $page->name); - $resp->assertHeader('Content-Disposition', 'attachment; filename="' . $page->slug . '.md"'); + $resp->assertHeader('Content-Disposition', 'attachment; filename*=UTF-8\'\'' . $page->slug . '.md'); } public function test_page_zip_endpoint() @@ -176,7 +176,7 @@ class ExportsApiTest extends TestCase $resp = $this->get("/api/pages/{$page->id}/export/zip"); $resp->assertStatus(200); - $resp->assertHeader('Content-Disposition', 'attachment; filename="' . $page->slug . '.zip"'); + $resp->assertHeader('Content-Disposition', 'attachment; filename*=UTF-8\'\'' . $page->slug . '.zip'); $zip = ZipTestHelper::extractFromZipResponse($resp); $this->assertArrayHasKey('page', $zip->data); diff --git a/tests/Exports/HtmlExportTest.php b/tests/Exports/HtmlExportTest.php index e039fb2cc..f23352e0e 100644 --- a/tests/Exports/HtmlExportTest.php +++ b/tests/Exports/HtmlExportTest.php @@ -18,7 +18,7 @@ class HtmlExportTest extends TestCase $resp = $this->get($page->getUrl('/export/html')); $resp->assertStatus(200); $resp->assertSee($page->name); - $resp->assertHeader('Content-Disposition', 'attachment; filename="' . $page->slug . '.html"'); + $resp->assertHeader('Content-Disposition', 'attachment; filename*=UTF-8\'\'' . $page->slug . '.html'); } public function test_book_html_export() @@ -31,7 +31,7 @@ class HtmlExportTest extends TestCase $resp->assertStatus(200); $resp->assertSee($book->name); $resp->assertSee($page->name); - $resp->assertHeader('Content-Disposition', 'attachment; filename="' . $book->slug . '.html"'); + $resp->assertHeader('Content-Disposition', 'attachment; filename*=UTF-8\'\'' . $book->slug . '.html'); } public function test_book_html_export_shows_html_descriptions() @@ -58,7 +58,7 @@ class HtmlExportTest extends TestCase $resp->assertStatus(200); $resp->assertSee($chapter->name); $resp->assertSee($page->name); - $resp->assertHeader('Content-Disposition', 'attachment; filename="' . $chapter->slug . '.html"'); + $resp->assertHeader('Content-Disposition', 'attachment; filename*=UTF-8\'\'' . $chapter->slug . '.html'); } public function test_chapter_html_export_shows_html_descriptions() diff --git a/tests/Exports/MarkdownExportTest.php b/tests/Exports/MarkdownExportTest.php index 09928ced2..ac1e283f3 100644 --- a/tests/Exports/MarkdownExportTest.php +++ b/tests/Exports/MarkdownExportTest.php @@ -14,7 +14,7 @@ class MarkdownExportTest extends TestCase $resp = $this->asEditor()->get($page->getUrl('/export/markdown')); $resp->assertStatus(200); $resp->assertSee($page->name); - $resp->assertHeader('Content-Disposition', 'attachment; filename="' . $page->slug . '.md"'); + $resp->assertHeader('Content-Disposition', 'attachment; filename*=UTF-8\'\'' . $page->slug . '.md'); } public function test_page_markdown_export_uses_existing_markdown_if_apparent() diff --git a/tests/Exports/PdfExportTest.php b/tests/Exports/PdfExportTest.php index e4de87d0d..f311f8457 100644 --- a/tests/Exports/PdfExportTest.php +++ b/tests/Exports/PdfExportTest.php @@ -17,7 +17,7 @@ class PdfExportTest extends TestCase $resp = $this->get($page->getUrl('/export/pdf')); $resp->assertStatus(200); - $resp->assertHeader('Content-Disposition', 'attachment; filename="' . $page->slug . '.pdf"'); + $resp->assertHeader('Content-Disposition', 'attachment; filename*=UTF-8\'\'' . $page->slug . '.pdf'); } public function test_book_pdf_export() @@ -28,7 +28,7 @@ class PdfExportTest extends TestCase $resp = $this->get($book->getUrl('/export/pdf')); $resp->assertStatus(200); - $resp->assertHeader('Content-Disposition', 'attachment; filename="' . $book->slug . '.pdf"'); + $resp->assertHeader('Content-Disposition', 'attachment; filename*=UTF-8\'\'' . $book->slug . '.pdf'); } public function test_chapter_pdf_export() @@ -38,7 +38,7 @@ class PdfExportTest extends TestCase $resp = $this->get($chapter->getUrl('/export/pdf')); $resp->assertStatus(200); - $resp->assertHeader('Content-Disposition', 'attachment; filename="' . $chapter->slug . '.pdf"'); + $resp->assertHeader('Content-Disposition', 'attachment; filename*=UTF-8\'\'' . $chapter->slug . '.pdf'); } diff --git a/tests/Exports/TextExportTest.php b/tests/Exports/TextExportTest.php index c593a6585..4b2d62887 100644 --- a/tests/Exports/TextExportTest.php +++ b/tests/Exports/TextExportTest.php @@ -14,7 +14,7 @@ class TextExportTest extends TestCase $resp = $this->get($page->getUrl('/export/plaintext')); $resp->assertStatus(200); $resp->assertSee($page->name); - $resp->assertHeader('Content-Disposition', 'attachment; filename="' . $page->slug . '.txt"'); + $resp->assertHeader('Content-Disposition', 'attachment; filename*=UTF-8\'\'' . $page->slug . '.txt'); } public function test_book_text_export() @@ -35,7 +35,7 @@ class TextExportTest extends TestCase $resp->assertSee($directPage->name); $resp->assertSee('My awesome page'); $resp->assertSee('My little nested page'); - $resp->assertHeader('Content-Disposition', 'attachment; filename="' . $book->slug . '.txt"'); + $resp->assertHeader('Content-Disposition', 'attachment; filename*=UTF-8\'\'' . $book->slug . '.txt'); } public function test_book_text_export_format() @@ -68,7 +68,7 @@ class TextExportTest extends TestCase $resp->assertSee($chapter->name); $resp->assertSee($page->name); $resp->assertSee('This is content within the page!'); - $resp->assertHeader('Content-Disposition', 'attachment; filename="' . $chapter->slug . '.txt"'); + $resp->assertHeader('Content-Disposition', 'attachment; filename*=UTF-8\'\'' . $chapter->slug . '.txt'); } public function test_chapter_text_export_format() diff --git a/tests/Uploads/AttachmentTest.php b/tests/Uploads/AttachmentTest.php index b443ca9f1..945fc258d 100644 --- a/tests/Uploads/AttachmentTest.php +++ b/tests/Uploads/AttachmentTest.php @@ -324,7 +324,7 @@ class AttachmentTest extends TestCase $attachmentGet = $this->get($attachment->getUrl(true)); // http-foundation/Response does some 'fixing' of responses to add charsets to text responses. $attachmentGet->assertHeader('Content-Type', 'text/plain; charset=utf-8'); - $attachmentGet->assertHeader('Content-Disposition', 'inline; filename="upload_test_file.txt"'); + $attachmentGet->assertHeader('Content-Disposition', 'inline; filename*=UTF-8\'\'upload_test_file.txt'); $attachmentGet->assertHeader('X-Content-Type-Options', 'nosniff'); $this->files->deleteAllAttachmentFiles(); @@ -340,7 +340,22 @@ class AttachmentTest extends TestCase $attachmentGet = $this->get($attachment->getUrl(true)); // http-foundation/Response does some 'fixing' of responses to add charsets to text responses. $attachmentGet->assertHeader('Content-Type', 'text/plain; charset=utf-8'); - $attachmentGet->assertHeader('Content-Disposition', 'inline; filename="test_file.html"'); + $attachmentGet->assertHeader('Content-Disposition', 'inline; filename*=UTF-8\'\'test_file.html'); + + $this->files->deleteAllAttachmentFiles(); + } + + public function test_file_access_name_in_content_disposition_header_is_sanitized() + { + $page = $this->entities->page(); + $this->asAdmin(); + + $attachment = $this->files->uploadAttachmentDataToPage($this, $page, 'test_file.html', '

testing

', 'text/html'); + $attachment->name = "my\\_/super\n_fu\$n_\tfile"; + $attachment->save(); + + $attachmentGet = $this->get($attachment->getUrl(true)); + $attachmentGet->assertHeader('Content-Disposition', 'inline; filename*=UTF-8\'\'my_super_fun_file.html'); $this->files->deleteAllAttachmentFiles(); }