From 684a94c4195e7d0c7b10268cd4a99717ad637710 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Sat, 11 Apr 2026 18:49:34 +0100 Subject: [PATCH] Theme Modules: Prevented zip-slip in new module extraction method Updated the new (development only) approach which could result in zip-slip causing trouble. This adds path normalisation, and testing to cover. --- app/Theming/ThemeModuleManager.php | 9 ++++++++- app/Theming/ThemeModuleZip.php | 8 +++++++- tests/Commands/InstallModuleCommandTest.php | 17 +++++++++++++++++ 3 files changed, 32 insertions(+), 2 deletions(-) diff --git a/app/Theming/ThemeModuleManager.php b/app/Theming/ThemeModuleManager.php index 86362a2f3..ed33014eb 100644 --- a/app/Theming/ThemeModuleManager.php +++ b/app/Theming/ThemeModuleManager.php @@ -51,7 +51,14 @@ class ThemeModuleManager } $folderPath = $this->modulesFolderPath . DIRECTORY_SEPARATOR . $folderName; - $zip->extractTo($folderPath); + try { + $zip->extractTo($folderPath); + } catch (ThemeModuleException $exception) { + if (is_dir($folderPath)) { + $this->deleteDirectoryRecursively($folderPath); + } + throw new ThemeModuleException("Failed to load extract files from module ZIP with error: {$exception->getMessage()}"); + } $module = $this->loadFromFolder($folderName); if (!$module) { diff --git a/app/Theming/ThemeModuleZip.php b/app/Theming/ThemeModuleZip.php index 4785abadb..7e94074c8 100644 --- a/app/Theming/ThemeModuleZip.php +++ b/app/Theming/ThemeModuleZip.php @@ -2,6 +2,7 @@ namespace BookStack\Theming; +use BookStack\Util\FilePathNormalizer; use ZipArchive; readonly class ThemeModuleZip @@ -33,7 +34,12 @@ readonly class ThemeModuleZip $name = str_replace($prefix, '', $name); } - $targetPath = $destinationPath . DIRECTORY_SEPARATOR . $name; + try { + $targetPath = $destinationPath . DIRECTORY_SEPARATOR . FilePathNormalizer::normalize($name); + } catch (\Exception $exception) { + throw new ThemeModuleException("Bad file path found in module ZIP file: {$name}"); + } + $targetPathDir = dirname($targetPath); if (!is_dir($targetPathDir)) { $dirCreated = mkdir($targetPathDir, 0777, true); diff --git a/tests/Commands/InstallModuleCommandTest.php b/tests/Commands/InstallModuleCommandTest.php index c085c4907..e96fc02c1 100644 --- a/tests/Commands/InstallModuleCommandTest.php +++ b/tests/Commands/InstallModuleCommandTest.php @@ -250,6 +250,23 @@ class InstallModuleCommandTest extends TestCase }); } + public function test_module_install_negates_zip_slip() + { + $this->usingThemeFolder(function () { + $zip = $this->getModuleZipPath(null, [ + '../parent.txt' => str_repeat('dog', 10) + ]); + + $expectedInstallPath = theme_path('modules/test-module'); + $this->artisan('bookstack:install-module', ['location' => $zip]) + ->expectsConfirmation('Are you sure you want to install this module?', 'yes') + ->expectsOutput("ERROR: Failed to install module with error: Failed to load extract files from module ZIP with error: Bad file path found in module ZIP file: ../parent.txt") + ->assertExitCode(1); + + $this->assertDirectoryDoesNotExist($expectedInstallPath); + }); + } + public function test_local_module_install_without_active_theme_can_setup_theme_folder() { $zip = $this->getModuleZipPath();