Merge pull request #5968 from BookStackApp/limits

Add some additional resource-based limits

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
      */

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)) {

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');

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);
         }

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<T>
+     */
+    public function limit(int $limit): self
+    {
+        return new self(array_slice(array_values($this->options), 0, $limit));
+    }
 }

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.
      */

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.',

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.',

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' => '<p>Hello</p>',
+                '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' => '<p>' . str_repeat('a', 2500000) . '</p>',
+            ],
+        ]);
+
+        $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);
+    }
 }

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());
+    }
 }