Skip to content

Commit 88d86df

Browse files
committed
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.
1 parent 38d3697 commit 88d86df

File tree

6 files changed

+89
-1
lines changed

6 files changed

+89
-1
lines changed

app/Exports/ZipExports/ZipExportReader.php

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,16 @@ public function readData(): array
5858
{
5959
$this->open();
6060

61+
$info = $this->zip->statName('data.json');
62+
if ($info === false) {
63+
throw new ZipExportException(trans('errors.import_zip_cant_decode_data'));
64+
}
65+
66+
$maxSize = max(intval(config()->get('app.upload_limit')), 1) * 1000000;
67+
if ($info['size'] > $maxSize) {
68+
throw new ZipExportException(trans('errors.import_zip_data_too_large'));
69+
}
70+
6171
// Validate json data exists, including metadata
6272
$jsonData = $this->zip->getFromName('data.json') ?: '';
6373
$importData = json_decode($jsonData, true);
@@ -73,6 +83,17 @@ public function fileExists(string $fileName): bool
7383
return $this->zip->statName("files/{$fileName}") !== false;
7484
}
7585

86+
public function fileWithinSizeLimit(string $fileName): bool
87+
{
88+
$fileInfo = $this->zip->statName("files/{$fileName}");
89+
if ($fileInfo === false) {
90+
return false;
91+
}
92+
93+
$maxSize = max(intval(config()->get('app.upload_limit')), 1) * 1000000;
94+
return $fileInfo['size'] <= $maxSize;
95+
}
96+
7697
/**
7798
* @return false|resource
7899
*/

app/Exports/ZipExports/ZipFileReferenceRule.php

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@ public function __construct(
1313
) {
1414
}
1515

16-
1716
/**
1817
* @inheritDoc
1918
*/
@@ -23,6 +22,13 @@ public function validate(string $attribute, mixed $value, Closure $fail): void
2322
$fail('validation.zip_file')->translate();
2423
}
2524

25+
if (!$this->context->zipReader->fileWithinSizeLimit($value)) {
26+
$fail('validation.zip_file_size')->translate([
27+
'attribute' => $value,
28+
'size' => config('app.upload_limit'),
29+
]);
30+
}
31+
2632
if (!empty($this->acceptedMimes)) {
2733
$fileMime = $this->context->zipReader->sniffFileMime($value);
2834
if (!in_array($fileMime, $this->acceptedMimes)) {

app/Exports/ZipExports/ZipImportRunner.php

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -265,6 +265,12 @@ protected function exportTagsToInputArray(array $exportTags): array
265265

266266
protected function zipFileToUploadedFile(string $fileName, ZipExportReader $reader): UploadedFile
267267
{
268+
if (!$reader->fileWithinSizeLimit($fileName)) {
269+
throw new ZipImportException([
270+
"File $fileName exceeds app upload limit."
271+
]);
272+
}
273+
268274
$tempPath = tempnam(sys_get_temp_dir(), 'bszipextract');
269275
$fileStream = $reader->streamFile($fileName);
270276
$tempStream = fopen($tempPath, 'wb');

lang/en/errors.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,7 @@
109109
'import_zip_cant_read' => 'Could not read ZIP file.',
110110
'import_zip_cant_decode_data' => 'Could not find and decode ZIP data.json content.',
111111
'import_zip_no_data' => 'ZIP file data has no expected book, chapter or page content.',
112+
'import_zip_data_too_large' => 'ZIP data.json content exceeds the configured application maximum upload size.',
112113
'import_validation_failed' => 'Import ZIP failed to validate with errors:',
113114
'import_zip_failed_notification' => 'Failed to import ZIP file.',
114115
'import_perms_books' => 'You are lacking the required permissions to create books.',

lang/en/validation.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,7 @@
106106
'uploaded' => 'The file could not be uploaded. The server may not accept files of this size.',
107107

108108
'zip_file' => 'The :attribute needs to reference a file within the ZIP.',
109+
'zip_file_size' => 'The file :attribute must not exceed :size MB.',
109110
'zip_file_mime' => 'The :attribute needs to reference a file of type :validTypes, found :foundType.',
110111
'zip_model_expected' => 'Data object expected but ":type" found.',
111112
'zip_unique' => 'The :attribute must be unique for the object type within the ZIP.',

tests/Exports/ZipImportRunnerTest.php

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
use BookStack\Entities\Models\Book;
66
use BookStack\Entities\Models\Chapter;
77
use BookStack\Entities\Models\Page;
8+
use BookStack\Exceptions\ZipImportException;
89
use BookStack\Exports\ZipExports\ZipImportRunner;
910
use BookStack\Uploads\Image;
1011
use Tests\TestCase;
@@ -431,4 +432,56 @@ public function test_drawing_references_are_updated_within_content()
431432

432433
ZipTestHelper::deleteZipForImport($import);
433434
}
435+
436+
public function test_error_thrown_if_zip_item_exceeds_app_file_upload_limit()
437+
{
438+
$tempFile = tempnam(sys_get_temp_dir(), 'bs-zip-test');
439+
file_put_contents($tempFile, str_repeat('a', 2500000));
440+
$parent = $this->entities->chapter();
441+
config()->set('app.upload_limit', 1);
442+
443+
$import = ZipTestHelper::importFromData([], [
444+
'page' => [
445+
'name' => 'Page A',
446+
'html' => '<p>Hello</p>',
447+
'attachments' => [
448+
[
449+
'name' => 'Text attachment',
450+
'file' => 'file_attachment'
451+
]
452+
],
453+
],
454+
], [
455+
'file_attachment' => $tempFile,
456+
]);
457+
458+
$this->asAdmin();
459+
460+
$this->expectException(ZipImportException::class);
461+
$this->expectExceptionMessage('The file file_attachment must not exceed 1 MB.');
462+
463+
$this->runner->run($import, $parent);
464+
ZipTestHelper::deleteZipForImport($import);
465+
}
466+
467+
public function test_error_thrown_if_zip_data_exceeds_app_file_upload_limit()
468+
{
469+
$parent = $this->entities->chapter();
470+
config()->set('app.upload_limit', 1);
471+
472+
$import = ZipTestHelper::importFromData([], [
473+
'page' => [
474+
'name' => 'Page A',
475+
'html' => '<p>' . str_repeat('a', 2500000) . '</p>',
476+
],
477+
]);
478+
479+
$this->asAdmin();
480+
481+
$this->expectException(ZipImportException::class);
482+
$this->expectExceptionMessage('ZIP data.json content exceeds the configured application maximum upload size.');
483+
484+
$this->runner->run($import, $parent);
485+
ZipTestHelper::deleteZipForImport($import);
486+
}
434487
}

0 commit comments

Comments
 (0)