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 c3d5c23cfec..28b830167c8 100644 --- a/app/Exports/ZipExports/ZipExportReader.php +++ b/app/Exports/ZipExports/ZipExportReader.php @@ -58,6 +58,16 @@ public function readData(): array { $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 @@ public function fileExists(string $fileName): bool 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 90e78c060b0..01d703a1d0a 100644 --- a/app/Exports/ZipExports/ZipFileReferenceRule.php +++ b/app/Exports/ZipExports/ZipFileReferenceRule.php @@ -13,7 +13,6 @@ public function __construct( ) { } - /** * @inheritDoc */ @@ -23,6 +22,13 @@ public function validate(string $attribute, mixed $value, Closure $fail): void $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 748acf43f74..382e4073eec 100644 --- a/app/Exports/ZipExports/ZipImportRunner.php +++ b/app/Exports/ZipExports/ZipImportRunner.php @@ -265,6 +265,12 @@ protected function exportTagsToInputArray(array $exportTags): array 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 9d738379648..77d7ee69e49 100644 --- a/lang/en/errors.php +++ b/lang/en/errors.php @@ -109,6 +109,7 @@ '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 d9b982d1e23..ff028525df3 100644 --- a/lang/en/validation.php +++ b/lang/en/validation.php @@ -106,6 +106,7 @@ '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 2255e16c393..67c1a90e528 100644 --- a/tests/Exports/ZipImportRunnerTest.php +++ b/tests/Exports/ZipImportRunnerTest.php @@ -5,6 +5,7 @@ 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 @@ public function test_drawing_references_are_updated_within_content() 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 8a6a5bbdedf..348d44a427f 100644 --- a/app/Search/SearchController.php +++ b/app/Search/SearchController.php @@ -78,8 +78,9 @@ public function searchForSelector(Request $request, QueryPopular $queryPopular) // 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 844d145e6fc..19f1c550976 100644 --- a/app/Search/SearchOptionSet.php +++ b/app/Search/SearchOptionSet.php @@ -82,4 +82,12 @@ public function nonNegated(): self $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 bf527d9c305..83af2d043d8 100644 --- a/app/Search/SearchOptions.php +++ b/app/Search/SearchOptions.php @@ -35,6 +35,7 @@ public static function fromString(string $search): self { $instance = new self(); $instance->addOptionsFromString($search); + $instance->limitOptions(); return $instance; } @@ -87,6 +88,8 @@ public static function fromRequest(Request $request): self $instance->filters = $instance->filters->merge($extras->filters); } + $instance->limitOptions(); + return $instance; } @@ -147,6 +150,25 @@ protected function addOptionsFromString(string $searchString): void $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 2ebf273dd55..4b0fa0f3aa4 100644 --- a/tests/Search/SearchOptionsTest.php +++ b/tests/Search/SearchOptionsTest.php @@ -142,4 +142,53 @@ public function test_from_request_properly_parses_out_extras_as_string() $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()); + } }