Confine asset combiner imports to allowed roots (GHSA-2223-f22x-24cq)

modules/cms/ServiceProvider.php

@@ -332,12 +332,14 @@ protected function registerBackendPermissions()
                 'cms.manage_content' => [
                     'label' => 'cms::lang.permissions.manage_content',
                     'tab' => 'cms::lang.permissions.name',
+                    'comment' => 'cms::lang.permissions.manage_content_comment',
                     'roles' => [UserRole::CODE_DEVELOPER],
                     'order' => 100
                 ],
                 'cms.manage_assets' => [
                     'label' => 'cms::lang.permissions.manage_assets',
                     'tab' => 'cms::lang.permissions.name',
+                    'comment' => 'cms::lang.permissions.manage_assets_comment',
                     'roles' => [UserRole::CODE_DEVELOPER],
                     'order' => 100
                 ],

modules/cms/lang/en/lang.php

@@ -276,7 +276,9 @@
     'permissions' => [
         'name' => 'CMS',
         'manage_content' => 'Manage website content files',
+        'manage_content_comment' => 'This permission should only be given to trusted users, as it allows direct access to the theme\'s content files.',
         'manage_assets' => 'Manage website assets - images, JavaScript files, CSS files',
+        'manage_assets_comment' => 'This permission should only be given to trusted users, as it allows direct access to the theme\'s asset files, which are combined and served publicly.',
         'manage_pages' => 'Create, modify and delete website pages',
         'manage_pages_comment' => 'This permission should only be given to trusted users, as it allows direct access to the theme\'s page content files, including PHP code if enabled.',
         'manage_layouts' => 'Create, modify and delete CMS layouts',

modules/system/classes/CombineAssets.php

@@ -18,6 +18,7 @@
 use Assetic\Filter\CssRewriteFilter;
 use Assetic\Filter\JavaScriptMinifierFilter;
 use Assetic\Filter\StylesheetMinifyFilter;
+use Winter\Storm\Filesystem\PathResolver;
 use Winter\Storm\Parse\Assetic\Cache\FilesystemCache;
 use Winter\Storm\Parse\Assetic\Filter\LessCompiler;
 use Winter\Storm\Parse\Assetic\Filter\ScssCompiler;
@@ -130,30 +131,44 @@ public function init()
             $this->useDeepHashing = Config::get('app.debug', false);
         }
 
+        // Constrain asset import directives to known asset trees. Without these
+        // explicit roots, a writable asset could disclose arbitrary server-readable
+        // files: `@import (inline) "<path>"` in a .less file (GHSA-58fp-mcx6-7qf9),
+        // `=include ../../../.env` in a .js file (GHSA-2223-f22x-24cq), or an
+        // `@import` traversal in a .css file. The asset's own source directory is
+        // always allowed implicitly; this list adds the cross-tree roots that
+        // legitimate themes/plugins/modules actually import from (e.g. a plugin
+        // importing a module asset, or a theme importing its own ../vendor).
+        $allowedImportRoots = [
+            themes_path(),
+            plugins_path(),
+            base_path('modules'),
+        ];
+
         /*
          * Register JavaScript filters
          */
-        $this->registerFilter('js', new JavascriptImporter);
+        $jsImporter = new JavascriptImporter;
+        $jsImporter->setAllowedImportRoots($allowedImportRoots);
+        $this->registerFilter('js', $jsImporter);
 
         /*
          * Register CSS filters
          */
-        $this->registerFilter('css', new CssImportFilter);
+        $cssImportFilter = new CssImportFilter;
+        // Assetic's CssImportFilter resolves `@import` targets relative to the source
+        // with `..` traversal allowed; confine the resolved path to the allowed roots.
+        $cssImportFilter->setImportValidator(function ($path) use ($allowedImportRoots) {
+            $resolved = PathResolver::resolve($path);
+
+            return $resolved !== false
+                && PathResolver::withinAny($resolved, $allowedImportRoots);
+        });
+        $this->registerFilter('css', $cssImportFilter);
         $this->registerFilter(['css', 'less', 'scss'], new CssRewriteFilter);
 
-        // Constrain @import resolution to known asset trees. Without these
-        // explicit roots, `@import (inline) "<path>"` in a writable .less file
-        // would disclose any server-readable file. The asset's own source root
-        // is always allowed implicitly; this list adds the cross-tree roots
-        // that legitimate themes/plugins actually use (e.g. a plugin importing
-        // a module .less, or a theme importing its own ../vendor). See
-        // GHSA-58fp-mcx6-7qf9.
         $lessCompiler = new LessCompiler;
-        $lessCompiler->setAllowedImportRoots([
-            themes_path(),
-            plugins_path(),
-            base_path('modules'),
-        ]);
+        $lessCompiler->setAllowedImportRoots($allowedImportRoots);
         $this->registerFilter('less', $lessCompiler);
         $this->registerFilter('scss', new ScssCompiler);
 

modules/system/tests/classes/CombineAssetsTest.php

@@ -169,6 +169,85 @@ public function testLessCompilerAllowsLegitimatePartial()
         }
     }
 
+    /**
+     * Regression for GHSA-2223-f22x-24cq. A writable theme `.js` asset containing
+     * `=include ../../../.env` must not disclose server files through the combiner,
+     * whose output is served unauthenticated via the `combine/{file}` route.
+     */
+    public function testJavascriptImporterBlocksTraversalImport()
+    {
+        $themeDir = $this->makeTempThemeDir();
+        // A real `.js` secret outside the theme subtree (but under base_path so
+        // Assetic's FileAsset root check passes). It escapes via `..` traversal but
+        // lands outside every allowed import root, so it must not be inlined.
+        $secretPath = dirname($themeDir) . '/js-secret-' . bin2hex(random_bytes(4)) . '.js';
+        file_put_contents($secretPath, 'var LEAK = "combine-leak-canary";');
+        file_put_contents(
+            $themeDir . '/assets/poc.js',
+            "/*\n=include ../../" . basename($secretPath) . "\n*/\n"
+        );
+
+        try {
+            $js = $this->compileJsTo($themeDir, 'assets/poc.js');
+            $this->assertStringNotContainsString('combine-leak-canary', $js);
+        } finally {
+            @unlink($secretPath);
+            \File::deleteDirectory($themeDir);
+        }
+    }
+
+    /**
+     * The `.js`-only extension gate must block disclosure of non-JS files (e.g.
+     * `.env`) before any path resolution, even inside an otherwise reachable tree.
+     */
+    public function testJavascriptImporterBlocksDisallowedExtension()
+    {
+        $themeDir = $this->makeTempThemeDir();
+        $secretPath = dirname($themeDir) . '/js-secret-' . bin2hex(random_bytes(4)) . '.env';
+        file_put_contents($secretPath, "APP_KEY=combine-leak-canary\n");
+        file_put_contents(
+            $themeDir . '/assets/poc.js',
+            "/*\n=include ../../" . basename($secretPath) . "\n*/\n"
+        );
+
+        try {
+            $js = $this->compileJsTo($themeDir, 'assets/poc.js');
+            $this->assertStringNotContainsString('combine-leak-canary', $js);
+        } finally {
+            @unlink($secretPath);
+            \File::deleteDirectory($themeDir);
+        }
+    }
+
+    /**
+     * Legitimate same-tree `=include partial.js` must still resolve, otherwise the
+     * hardening would break every asset that composes its own bundle.
+     */
+    public function testJavascriptImporterAllowsSameTreeInclude()
+    {
+        $themeDir = $this->makeTempThemeDir();
+        file_put_contents($themeDir . '/assets/partial.js', 'var PARTIAL = "partial-marker";');
+        file_put_contents($themeDir . '/assets/main.js', "/*\n=include partial.js\n*/\nvar MAIN = 1;");
+
+        try {
+            $js = $this->compileJsTo($themeDir, 'assets/main.js');
+            $this->assertStringContainsString('partial-marker', $js);
+        } finally {
+            \File::deleteDirectory($themeDir);
+        }
+    }
+
+    protected function compileJsTo(string $themeDir, string $relativeAsset): string
+    {
+        $dest = sys_get_temp_dir() . '/winter-combine-out-' . bin2hex(random_bytes(4)) . '.js';
+        try {
+            CombineAssets::instance()->combineToFile([$relativeAsset], $dest, $themeDir);
+            return file_get_contents($dest) ?: '';
+        } finally {
+            @unlink($dest);
+        }
+    }
+
     /** @var string */
     protected $lastSecretPath = '';