From 88d86df66f3eca1e0cd67c052aa1a99116d4d557 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Mon, 29 Dec 2025 23:08:18 +0000 Subject: [PATCH 1/2] ZIP Exports: Added limit to ZIP file size before extraction Checks files within the ZIP again the app upload file limit before using/streaming/extracting, to help ensure that they do no exceed what might be expected on that instance, and to prevent disk exhaustion via things like super high compression ratio files. Thanks to Jeong Woo Lee (eclipse07077-ljw) for reporting. --- app/Exports/ZipExports/ZipExportReader.php | 21 ++++++++ .../ZipExports/ZipFileReferenceRule.php | 8 ++- app/Exports/ZipExports/ZipImportRunner.php | 6 +++ lang/en/errors.php | 1 + lang/en/validation.php | 1 + tests/Exports/ZipImportRunnerTest.php | 53 +++++++++++++++++++ 6 files changed, 89 insertions(+), 1 deletion(-) diff --git a/app/Exports/ZipExports/ZipExportReader.php b/app/Exports/ZipExports/ZipExportReader.php index c3d5c23cf..28b830167 100644 --- a/app/Exports/ZipExports/ZipExportReader.php +++ b/app/Exports/ZipExports/ZipExportReader.php @@ -58,6 +58,16 @@ class ZipExportReader { $this->open(); + $info = $this->zip->statName('data.json'); + if ($info === false) { + throw new ZipExportException(trans('errors.import_zip_cant_decode_data')); + } + + $maxSize = max(intval(config()->get('app.upload_limit')), 1) * 1000000; + if ($info['size'] > $maxSize) { + throw new ZipExportException(trans('errors.import_zip_data_too_large')); + } + // Validate json data exists, including metadata $jsonData = $this->zip->getFromName('data.json') ?: ''; $importData = json_decode($jsonData, true); @@ -73,6 +83,17 @@ class ZipExportReader return $this->zip->statName("files/{$fileName}") !== false; } + public function fileWithinSizeLimit(string $fileName): bool + { + $fileInfo = $this->zip->statName("files/{$fileName}"); + if ($fileInfo === false) { + return false; + } + + $maxSize = max(intval(config()->get('app.upload_limit')), 1) * 1000000; + return $fileInfo['size'] <= $maxSize; + } + /** * @return false|resource */ diff --git a/app/Exports/ZipExports/ZipFileReferenceRule.php b/app/Exports/ZipExports/ZipFileReferenceRule.php index 90e78c060..01d703a1d 100644 --- a/app/Exports/ZipExports/ZipFileReferenceRule.php +++ b/app/Exports/ZipExports/ZipFileReferenceRule.php @@ -13,7 +13,6 @@ class ZipFileReferenceRule implements ValidationRule ) { } - /** * @inheritDoc */ @@ -23,6 +22,13 @@ class ZipFileReferenceRule implements ValidationRule $fail('validation.zip_file')->translate(); } + if (!$this->context->zipReader->fileWithinSizeLimit($value)) { + $fail('validation.zip_file_size')->translate([ + 'attribute' => $value, + 'size' => config('app.upload_limit'), + ]); + } + if (!empty($this->acceptedMimes)) { $fileMime = $this->context->zipReader->sniffFileMime($value); if (!in_array($fileMime, $this->acceptedMimes)) { diff --git a/app/Exports/ZipExports/ZipImportRunner.php b/app/Exports/ZipExports/ZipImportRunner.php index 748acf43f..382e4073e 100644 --- a/app/Exports/ZipExports/ZipImportRunner.php +++ b/app/Exports/ZipExports/ZipImportRunner.php @@ -265,6 +265,12 @@ class ZipImportRunner protected function zipFileToUploadedFile(string $fileName, ZipExportReader $reader): UploadedFile { + if (!$reader->fileWithinSizeLimit($fileName)) { + throw new ZipImportException([ + "File $fileName exceeds app upload limit." + ]); + } + $tempPath = tempnam(sys_get_temp_dir(), 'bszipextract'); $fileStream = $reader->streamFile($fileName); $tempStream = fopen($tempPath, 'wb'); diff --git a/lang/en/errors.php b/lang/en/errors.php index 9d7383796..77d7ee69e 100644 --- a/lang/en/errors.php +++ b/lang/en/errors.php @@ -109,6 +109,7 @@ return [ 'import_zip_cant_read' => 'Could not read ZIP file.', 'import_zip_cant_decode_data' => 'Could not find and decode ZIP data.json content.', 'import_zip_no_data' => 'ZIP file data has no expected book, chapter or page content.', + 'import_zip_data_too_large' => 'ZIP data.json content exceeds the configured application maximum upload size.', 'import_validation_failed' => 'Import ZIP failed to validate with errors:', 'import_zip_failed_notification' => 'Failed to import ZIP file.', 'import_perms_books' => 'You are lacking the required permissions to create books.', diff --git a/lang/en/validation.php b/lang/en/validation.php index d9b982d1e..ff028525d 100644 --- a/lang/en/validation.php +++ b/lang/en/validation.php @@ -106,6 +106,7 @@ return [ 'uploaded' => 'The file could not be uploaded. The server may not accept files of this size.', 'zip_file' => 'The :attribute needs to reference a file within the ZIP.', + 'zip_file_size' => 'The file :attribute must not exceed :size MB.', 'zip_file_mime' => 'The :attribute needs to reference a file of type :validTypes, found :foundType.', 'zip_model_expected' => 'Data object expected but ":type" found.', 'zip_unique' => 'The :attribute must be unique for the object type within the ZIP.', diff --git a/tests/Exports/ZipImportRunnerTest.php b/tests/Exports/ZipImportRunnerTest.php index 2255e16c3..67c1a90e5 100644 --- a/tests/Exports/ZipImportRunnerTest.php +++ b/tests/Exports/ZipImportRunnerTest.php @@ -5,6 +5,7 @@ namespace Tests\Exports; use BookStack\Entities\Models\Book; use BookStack\Entities\Models\Chapter; use BookStack\Entities\Models\Page; +use BookStack\Exceptions\ZipImportException; use BookStack\Exports\ZipExports\ZipImportRunner; use BookStack\Uploads\Image; use Tests\TestCase; @@ -431,4 +432,56 @@ class ZipImportRunnerTest extends TestCase ZipTestHelper::deleteZipForImport($import); } + + public function test_error_thrown_if_zip_item_exceeds_app_file_upload_limit() + { + $tempFile = tempnam(sys_get_temp_dir(), 'bs-zip-test'); + file_put_contents($tempFile, str_repeat('a', 2500000)); + $parent = $this->entities->chapter(); + config()->set('app.upload_limit', 1); + + $import = ZipTestHelper::importFromData([], [ + 'page' => [ + 'name' => 'Page A', + 'html' => '

Hello

', + 'attachments' => [ + [ + 'name' => 'Text attachment', + 'file' => 'file_attachment' + ] + ], + ], + ], [ + 'file_attachment' => $tempFile, + ]); + + $this->asAdmin(); + + $this->expectException(ZipImportException::class); + $this->expectExceptionMessage('The file file_attachment must not exceed 1 MB.'); + + $this->runner->run($import, $parent); + ZipTestHelper::deleteZipForImport($import); + } + + public function test_error_thrown_if_zip_data_exceeds_app_file_upload_limit() + { + $parent = $this->entities->chapter(); + config()->set('app.upload_limit', 1); + + $import = ZipTestHelper::importFromData([], [ + 'page' => [ + 'name' => 'Page A', + 'html' => '

' . str_repeat('a', 2500000) . '

', + ], + ]); + + $this->asAdmin(); + + $this->expectException(ZipImportException::class); + $this->expectExceptionMessage('ZIP data.json content exceeds the configured application maximum upload size.'); + + $this->runner->run($import, $parent); + ZipTestHelper::deleteZipForImport($import); + } } From b08d1b36de36d96fae55fff65bcb5908a43e63b5 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Tue, 30 Dec 2025 13:29:04 +0000 Subject: [PATCH 2/2] Search: Set limits on the amount of search terms Sets some reasonable limits, which are higher when logged in since that infers a little extra trust. Helps prevent against large resource consuption attacks via super heavy search queries. Thanks to Gabriel Rodrigues AKA TEXUGO for reporting. --- app/Search/SearchController.php | 5 +-- app/Search/SearchOptionSet.php | 8 +++++ app/Search/SearchOptions.php | 22 ++++++++++++++ tests/Search/SearchOptionsTest.php | 49 ++++++++++++++++++++++++++++++ 4 files changed, 82 insertions(+), 2 deletions(-) diff --git a/app/Search/SearchController.php b/app/Search/SearchController.php index 8a6a5bbde..348d44a42 100644 --- a/app/Search/SearchController.php +++ b/app/Search/SearchController.php @@ -78,8 +78,9 @@ class SearchController extends Controller // Search for entities otherwise show most popular if ($searchTerm !== false) { - $searchTerm .= ' {type:' . implode('|', $entityTypes) . '}'; - $entities = $this->searchRunner->searchEntities(SearchOptions::fromString($searchTerm), 'all', 1, 20)['results']; + $options = SearchOptions::fromString($searchTerm); + $options->setFilter('type', implode('|', $entityTypes)); + $entities = $this->searchRunner->searchEntities($options, 'all', 1, 20)['results']; } else { $entities = $queryPopular->run(20, 0, $entityTypes); } diff --git a/app/Search/SearchOptionSet.php b/app/Search/SearchOptionSet.php index 844d145e6..19f1c5509 100644 --- a/app/Search/SearchOptionSet.php +++ b/app/Search/SearchOptionSet.php @@ -82,4 +82,12 @@ class SearchOptionSet $values = array_values(array_filter($this->options, fn (SearchOption $option) => !$option->negated)); return new self($values); } + + /** + * @return self + */ + public function limit(int $limit): self + { + return new self(array_slice(array_values($this->options), 0, $limit)); + } } diff --git a/app/Search/SearchOptions.php b/app/Search/SearchOptions.php index bf527d9c3..83af2d043 100644 --- a/app/Search/SearchOptions.php +++ b/app/Search/SearchOptions.php @@ -35,6 +35,7 @@ class SearchOptions { $instance = new self(); $instance->addOptionsFromString($search); + $instance->limitOptions(); return $instance; } @@ -87,6 +88,8 @@ class SearchOptions $instance->filters = $instance->filters->merge($extras->filters); } + $instance->limitOptions(); + return $instance; } @@ -147,6 +150,25 @@ class SearchOptions $this->filters = $this->filters->merge(new SearchOptionSet($terms['filters'])); } + /** + * Limit the amount of search options to reasonable levels. + * Provides higher limits to logged-in users since that signals a slightly + * higher level of trust. + */ + protected function limitOptions(): void + { + $userLoggedIn = !user()->isGuest(); + $searchLimit = $userLoggedIn ? 10 : 5; + $exactLimit = $userLoggedIn ? 4 : 2; + $tagLimit = $userLoggedIn ? 8 : 4; + $filterLimit = $userLoggedIn ? 10 : 5; + + $this->searches = $this->searches->limit($searchLimit); + $this->exacts = $this->exacts->limit($exactLimit); + $this->tags = $this->tags->limit($tagLimit); + $this->filters = $this->filters->limit($filterLimit); + } + /** * Decode backslash escaping within the input string. */ diff --git a/tests/Search/SearchOptionsTest.php b/tests/Search/SearchOptionsTest.php index 2ebf273dd..4b0fa0f3a 100644 --- a/tests/Search/SearchOptionsTest.php +++ b/tests/Search/SearchOptionsTest.php @@ -142,4 +142,53 @@ class SearchOptionsTest extends TestCase $this->assertEquals('dino', $options->exacts->all()[0]->value); $this->assertTrue($options->exacts->all()[0]->negated); } + + public function test_from_string_results_are_count_limited_and_larger_for_logged_in_users() + { + $terms = [ + ...array_fill(0, 40, 'cat'), + ...array_fill(0, 50, '"bees"'), + ...array_fill(0, 50, '{is_template}'), + ...array_fill(0, 50, '[a=b]'), + ]; + + $options = SearchOptions::fromString(implode(' ', $terms)); + + $this->assertCount(5, $options->searches->all()); + $this->assertCount(2, $options->exacts->all()); + $this->assertCount(4, $options->tags->all()); + $this->assertCount(5, $options->filters->all()); + + $this->asEditor(); + $options = SearchOptions::fromString(implode(' ', $terms)); + + $this->assertCount(10, $options->searches->all()); + $this->assertCount(4, $options->exacts->all()); + $this->assertCount(8, $options->tags->all()); + $this->assertCount(10, $options->filters->all()); + } + + public function test_from_request_results_are_count_limited_and_larger_for_logged_in_users() + { + $request = new Request([ + 'search' => str_repeat('hello ', 20), + 'tags' => array_fill(0, 20, 'a=b'), + 'extras' => str_repeat('-[b=c] -{viewed_by_me} -"dino"', 20), + ]); + + $options = SearchOptions::fromRequest($request); + + $this->assertCount(5, $options->searches->all()); + $this->assertCount(2, $options->exacts->all()); + $this->assertCount(4, $options->tags->all()); + $this->assertCount(5, $options->filters->all()); + + $this->asEditor(); + $options = SearchOptions::fromRequest($request); + + $this->assertCount(10, $options->searches->all()); + $this->assertCount(4, $options->exacts->all()); + $this->assertCount(8, $options->tags->all()); + $this->assertCount(10, $options->filters->all()); + } }