Skip to content

Commit a9ffd3e

Browse files
ssddanbrownssddanbrown
committed
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.
1 parent f4c9d2b commit a9ffd3e

File tree

7 files changed

+47
-29
lines changed

7 files changed

+47
-29
lines changed

‎app/Http/DownloadResponseFactory.php‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -102,12 +102,15 @@ public function streamedFileInline(string $filePath, ?string $fileName = null):
102102
protected function getHeaders(string $fileName, int $fileSize, string $mime = 'application/octet-stream'): array
103103
{
104104
$disposition = ($mime === 'application/octet-stream') ? 'attachment' : 'inline';
105-
$downloadName = str_replace('"', '', $fileName);
105+
106+
$downloadName = str_replace(['"', '/', '\\', '$'], '', $fileName);
107+
$downloadName = preg_replace('/[\x00-\x1F\x7F]/', '', $downloadName);
108+
$encodedDownloadName = rawurlencode($downloadName);
106109

107110
return [
108111
'Content-Type' => $mime,
109112
'Content-Length' => $fileSize,
110-
'Content-Disposition' => "{$disposition}; filename=\"{$downloadName}\"",
113+
'Content-Disposition' => "{$disposition}; filename*=UTF-8''{$encodedDownloadName}",
111114
'X-Content-Type-Options' => 'nosniff',
112115
];
113116
}

‎tests/Api/ExportsApiTest.php‎

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ public function test_book_html_endpoint()
1919
$resp = $this->get("/api/books/{$book->id}/export/html");
2020
$resp->assertStatus(200);
2121
$resp->assertSee($book->name);
22-
$resp->assertHeader('Content-Disposition', 'attachment; filename="' . $book->slug . '.html"');
22+
$resp->assertHeader('Content-Disposition', 'attachment; filename*=UTF-8\'\'' . $book->slug . '.html');
2323
}
2424

2525
public function test_book_plain_text_endpoint()
@@ -30,7 +30,7 @@ public function test_book_plain_text_endpoint()
3030
$resp = $this->get("/api/books/{$book->id}/export/plaintext");
3131
$resp->assertStatus(200);
3232
$resp->assertSee($book->name);
33-
$resp->assertHeader('Content-Disposition', 'attachment; filename="' . $book->slug . '.txt"');
33+
$resp->assertHeader('Content-Disposition', 'attachment; filename*=UTF-8\'\'' . $book->slug . '.txt');
3434
}
3535

3636
public function test_book_pdf_endpoint()
@@ -40,7 +40,7 @@ public function test_book_pdf_endpoint()
4040

4141
$resp = $this->get("/api/books/{$book->id}/export/pdf");
4242
$resp->assertStatus(200);
43-
$resp->assertHeader('Content-Disposition', 'attachment; filename="' . $book->slug . '.pdf"');
43+
$resp->assertHeader('Content-Disposition', 'attachment; filename*=UTF-8\'\'' . $book->slug . '.pdf');
4444
}
4545

4646
public function test_book_markdown_endpoint()
@@ -50,7 +50,7 @@ public function test_book_markdown_endpoint()
5050

5151
$resp = $this->get("/api/books/{$book->id}/export/markdown");
5252
$resp->assertStatus(200);
53-
$resp->assertHeader('Content-Disposition', 'attachment; filename="' . $book->slug . '.md"');
53+
$resp->assertHeader('Content-Disposition', 'attachment; filename*=UTF-8\'\'' . $book->slug . '.md');
5454
$resp->assertSee('# ' . $book->name);
5555
$resp->assertSee('# ' . $book->pages()->first()->name);
5656
$resp->assertSee('# ' . $book->chapters()->first()->name);
@@ -63,7 +63,7 @@ public function test_book_zip_endpoint()
6363

6464
$resp = $this->get("/api/books/{$book->id}/export/zip");
6565
$resp->assertStatus(200);
66-
$resp->assertHeader('Content-Disposition', 'attachment; filename="' . $book->slug . '.zip"');
66+
$resp->assertHeader('Content-Disposition', 'attachment; filename*=UTF-8\'\'' . $book->slug . '.zip');
6767

6868
$zip = ZipTestHelper::extractFromZipResponse($resp);
6969
$this->assertArrayHasKey('book', $zip->data);
@@ -77,7 +77,7 @@ public function test_chapter_html_endpoint()
7777
$resp = $this->get("/api/chapters/{$chapter->id}/export/html");
7878
$resp->assertStatus(200);
7979
$resp->assertSee($chapter->name);
80-
$resp->assertHeader('Content-Disposition', 'attachment; filename="' . $chapter->slug . '.html"');
80+
$resp->assertHeader('Content-Disposition', 'attachment; filename*=UTF-8\'\'' . $chapter->slug . '.html');
8181
}
8282

8383
public function test_chapter_plain_text_endpoint()
@@ -88,7 +88,7 @@ public function test_chapter_plain_text_endpoint()
8888
$resp = $this->get("/api/chapters/{$chapter->id}/export/plaintext");
8989
$resp->assertStatus(200);
9090
$resp->assertSee($chapter->name);
91-
$resp->assertHeader('Content-Disposition', 'attachment; filename="' . $chapter->slug . '.txt"');
91+
$resp->assertHeader('Content-Disposition', 'attachment; filename*=UTF-8\'\'' . $chapter->slug . '.txt');
9292
}
9393

9494
public function test_chapter_pdf_endpoint()
@@ -98,7 +98,7 @@ public function test_chapter_pdf_endpoint()
9898

9999
$resp = $this->get("/api/chapters/{$chapter->id}/export/pdf");
100100
$resp->assertStatus(200);
101-
$resp->assertHeader('Content-Disposition', 'attachment; filename="' . $chapter->slug . '.pdf"');
101+
$resp->assertHeader('Content-Disposition', 'attachment; filename*=UTF-8\'\'' . $chapter->slug . '.pdf');
102102
}
103103

104104
public function test_chapter_markdown_endpoint()
@@ -108,7 +108,7 @@ public function test_chapter_markdown_endpoint()
108108

109109
$resp = $this->get("/api/chapters/{$chapter->id}/export/markdown");
110110
$resp->assertStatus(200);
111-
$resp->assertHeader('Content-Disposition', 'attachment; filename="' . $chapter->slug . '.md"');
111+
$resp->assertHeader('Content-Disposition', 'attachment; filename*=UTF-8\'\'' . $chapter->slug . '.md');
112112
$resp->assertSee('# ' . $chapter->name);
113113
$resp->assertSee('# ' . $chapter->pages()->first()->name);
114114
}
@@ -120,7 +120,7 @@ public function test_chapter_zip_endpoint()
120120

121121
$resp = $this->get("/api/chapters/{$chapter->id}/export/zip");
122122
$resp->assertStatus(200);
123-
$resp->assertHeader('Content-Disposition', 'attachment; filename="' . $chapter->slug . '.zip"');
123+
$resp->assertHeader('Content-Disposition', 'attachment; filename*=UTF-8\'\'' . $chapter->slug . '.zip');
124124

125125
$zip = ZipTestHelper::extractFromZipResponse($resp);
126126
$this->assertArrayHasKey('chapter', $zip->data);
@@ -134,7 +134,7 @@ public function test_page_html_endpoint()
134134
$resp = $this->get("/api/pages/{$page->id}/export/html");
135135
$resp->assertStatus(200);
136136
$resp->assertSee($page->name);
137-
$resp->assertHeader('Content-Disposition', 'attachment; filename="' . $page->slug . '.html"');
137+
$resp->assertHeader('Content-Disposition', 'attachment; filename*=UTF-8\'\'' . $page->slug . '.html');
138138
}
139139

140140
public function test_page_plain_text_endpoint()
@@ -145,7 +145,7 @@ public function test_page_plain_text_endpoint()
145145
$resp = $this->get("/api/pages/{$page->id}/export/plaintext");
146146
$resp->assertStatus(200);
147147
$resp->assertSee($page->name);
148-
$resp->assertHeader('Content-Disposition', 'attachment; filename="' . $page->slug . '.txt"');
148+
$resp->assertHeader('Content-Disposition', 'attachment; filename*=UTF-8\'\'' . $page->slug . '.txt');
149149
}
150150

151151
public function test_page_pdf_endpoint()
@@ -155,7 +155,7 @@ public function test_page_pdf_endpoint()
155155

156156
$resp = $this->get("/api/pages/{$page->id}/export/pdf");
157157
$resp->assertStatus(200);
158-
$resp->assertHeader('Content-Disposition', 'attachment; filename="' . $page->slug . '.pdf"');
158+
$resp->assertHeader('Content-Disposition', 'attachment; filename*=UTF-8\'\'' . $page->slug . '.pdf');
159159
}
160160

161161
public function test_page_markdown_endpoint()
@@ -166,7 +166,7 @@ public function test_page_markdown_endpoint()
166166
$resp = $this->get("/api/pages/{$page->id}/export/markdown");
167167
$resp->assertStatus(200);
168168
$resp->assertSee('# ' . $page->name);
169-
$resp->assertHeader('Content-Disposition', 'attachment; filename="' . $page->slug . '.md"');
169+
$resp->assertHeader('Content-Disposition', 'attachment; filename*=UTF-8\'\'' . $page->slug . '.md');
170170
}
171171

172172
public function test_page_zip_endpoint()
@@ -176,7 +176,7 @@ public function test_page_zip_endpoint()
176176

177177
$resp = $this->get("/api/pages/{$page->id}/export/zip");
178178
$resp->assertStatus(200);
179-
$resp->assertHeader('Content-Disposition', 'attachment; filename="' . $page->slug . '.zip"');
179+
$resp->assertHeader('Content-Disposition', 'attachment; filename*=UTF-8\'\'' . $page->slug . '.zip');
180180

181181
$zip = ZipTestHelper::extractFromZipResponse($resp);
182182
$this->assertArrayHasKey('page', $zip->data);

‎tests/Exports/HtmlExportTest.php‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ public function test_page_html_export()
1818
$resp = $this->get($page->getUrl('/export/html'));
1919
$resp->assertStatus(200);
2020
$resp->assertSee($page->name);
21-
$resp->assertHeader('Content-Disposition', 'attachment; filename="' . $page->slug . '.html"');
21+
$resp->assertHeader('Content-Disposition', 'attachment; filename*=UTF-8\'\'' . $page->slug . '.html');
2222
}
2323

2424
public function test_book_html_export()
@@ -31,7 +31,7 @@ public function test_book_html_export()
3131
$resp->assertStatus(200);
3232
$resp->assertSee($book->name);
3333
$resp->assertSee($page->name);
34-
$resp->assertHeader('Content-Disposition', 'attachment; filename="' . $book->slug . '.html"');
34+
$resp->assertHeader('Content-Disposition', 'attachment; filename*=UTF-8\'\'' . $book->slug . '.html');
3535
}
3636

3737
public function test_book_html_export_shows_html_descriptions()
@@ -58,7 +58,7 @@ public function test_chapter_html_export()
5858
$resp->assertStatus(200);
5959
$resp->assertSee($chapter->name);
6060
$resp->assertSee($page->name);
61-
$resp->assertHeader('Content-Disposition', 'attachment; filename="' . $chapter->slug . '.html"');
61+
$resp->assertHeader('Content-Disposition', 'attachment; filename*=UTF-8\'\'' . $chapter->slug . '.html');
6262
}
6363

6464
public function test_chapter_html_export_shows_html_descriptions()

‎tests/Exports/MarkdownExportTest.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ public function test_page_markdown_export()
1414
$resp = $this->asEditor()->get($page->getUrl('/export/markdown'));
1515
$resp->assertStatus(200);
1616
$resp->assertSee($page->name);
17-
$resp->assertHeader('Content-Disposition', 'attachment; filename="' . $page->slug . '.md"');
17+
$resp->assertHeader('Content-Disposition', 'attachment; filename*=UTF-8\'\'' . $page->slug . '.md');
1818
}
1919

2020
public function test_page_markdown_export_uses_existing_markdown_if_apparent()

‎tests/Exports/PdfExportTest.php‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ public function test_page_pdf_export()
1717

1818
$resp = $this->get($page->getUrl('/export/pdf'));
1919
$resp->assertStatus(200);
20-
$resp->assertHeader('Content-Disposition', 'attachment; filename="' . $page->slug . '.pdf"');
20+
$resp->assertHeader('Content-Disposition', 'attachment; filename*=UTF-8\'\'' . $page->slug . '.pdf');
2121
}
2222

2323
public function test_book_pdf_export()
@@ -28,7 +28,7 @@ public function test_book_pdf_export()
2828

2929
$resp = $this->get($book->getUrl('/export/pdf'));
3030
$resp->assertStatus(200);
31-
$resp->assertHeader('Content-Disposition', 'attachment; filename="' . $book->slug . '.pdf"');
31+
$resp->assertHeader('Content-Disposition', 'attachment; filename*=UTF-8\'\'' . $book->slug . '.pdf');
3232
}
3333

3434
public function test_chapter_pdf_export()
@@ -38,7 +38,7 @@ public function test_chapter_pdf_export()
3838

3939
$resp = $this->get($chapter->getUrl('/export/pdf'));
4040
$resp->assertStatus(200);
41-
$resp->assertHeader('Content-Disposition', 'attachment; filename="' . $chapter->slug . '.pdf"');
41+
$resp->assertHeader('Content-Disposition', 'attachment; filename*=UTF-8\'\'' . $chapter->slug . '.pdf');
4242
}
4343

4444

‎tests/Exports/TextExportTest.php‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ public function test_page_text_export()
1414
$resp = $this->get($page->getUrl('/export/plaintext'));
1515
$resp->assertStatus(200);
1616
$resp->assertSee($page->name);
17-
$resp->assertHeader('Content-Disposition', 'attachment; filename="' . $page->slug . '.txt"');
17+
$resp->assertHeader('Content-Disposition', 'attachment; filename*=UTF-8\'\'' . $page->slug . '.txt');
1818
}
1919

2020
public function test_book_text_export()
@@ -35,7 +35,7 @@ public function test_book_text_export()
3535
$resp->assertSee($directPage->name);
3636
$resp->assertSee('My awesome page');
3737
$resp->assertSee('My little nested page');
38-
$resp->assertHeader('Content-Disposition', 'attachment; filename="' . $book->slug . '.txt"');
38+
$resp->assertHeader('Content-Disposition', 'attachment; filename*=UTF-8\'\'' . $book->slug . '.txt');
3939
}
4040

4141
public function test_book_text_export_format()
@@ -68,7 +68,7 @@ public function test_chapter_text_export()
6868
$resp->assertSee($chapter->name);
6969
$resp->assertSee($page->name);
7070
$resp->assertSee('This is content within the page!');
71-
$resp->assertHeader('Content-Disposition', 'attachment; filename="' . $chapter->slug . '.txt"');
71+
$resp->assertHeader('Content-Disposition', 'attachment; filename*=UTF-8\'\'' . $chapter->slug . '.txt');
7272
}
7373

7474
public function test_chapter_text_export_format()

‎tests/Uploads/AttachmentTest.php‎

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -324,7 +324,7 @@ public function test_file_access_with_open_query_param_provides_inline_response_
324324
$attachmentGet = $this->get($attachment->getUrl(true));
325325
// http-foundation/Response does some 'fixing' of responses to add charsets to text responses.
326326
$attachmentGet->assertHeader('Content-Type', 'text/plain; charset=utf-8');
327-
$attachmentGet->assertHeader('Content-Disposition', 'inline; filename="upload_test_file.txt"');
327+
$attachmentGet->assertHeader('Content-Disposition', 'inline; filename*=UTF-8\'\'upload_test_file.txt');
328328
$attachmentGet->assertHeader('X-Content-Type-Options', 'nosniff');
329329

330330
$this->files->deleteAllAttachmentFiles();
@@ -340,7 +340,22 @@ public function test_html_file_access_with_open_forces_plain_content_type()
340340
$attachmentGet = $this->get($attachment->getUrl(true));
341341
// http-foundation/Response does some 'fixing' of responses to add charsets to text responses.
342342
$attachmentGet->assertHeader('Content-Type', 'text/plain; charset=utf-8');
343-
$attachmentGet->assertHeader('Content-Disposition', 'inline; filename="test_file.html"');
343+
$attachmentGet->assertHeader('Content-Disposition', 'inline; filename*=UTF-8\'\'test_file.html');
344+
345+
$this->files->deleteAllAttachmentFiles();
346+
}
347+
348+
public function test_file_access_name_in_content_disposition_header_is_sanitized()
349+
{
350+
$page = $this->entities->page();
351+
$this->asAdmin();
352+
353+
$attachment = $this->files->uploadAttachmentDataToPage($this, $page, 'test_file.html', '<html></html><p>testing</p>', 'text/html');
354+
$attachment->name = "my\\_/super\n_fu\$n_\tfile";
355+
$attachment->save();
356+
357+
$attachmentGet = $this->get($attachment->getUrl(true));
358+
$attachmentGet->assertHeader('Content-Disposition', 'inline; filename*=UTF-8\'\'my_super_fun_file.html');
344359

345360
$this->files->deleteAllAttachmentFiles();
346361
}

0 commit comments

Comments
 (0)