Confine JavaScript importer paths to allowed roots (GHSA-2223-f22x-24cq)

composer.json

@@ -35,7 +35,7 @@
         "ext-pdo": "*",
         "ext-zip": "*",
 
-        "assetic/framework": "~3.0",
+        "assetic/framework": "^3.2.1",
         "doctrine/dbal": "^2.6",
         "enshrined/svg-sanitize": "~0.16",
         "laravel/framework": "^9.49",

src/Filesystem/PathResolver.php

@@ -122,6 +122,26 @@ public static function within(string $path, string $directory): bool
             || starts_with($path, rtrim($directory, '/\\') . '/');
     }
 
+    /**
+     * Determines whether a path is within any of the given directories.
+     *
+     * Returns true as soon as {@see within()} matches one of $directories. Empty and
+     * non-string entries are ignored, so callers may pass an optional context
+     * directory (which may be null) alongside a list of roots without pre-filtering.
+     *
+     * @param string[] $directories
+     */
+    public static function withinAny(string $path, array $directories): bool
+    {
+        foreach ($directories as $directory) {
+            if (is_string($directory) && $directory !== '' && static::within($path, $directory)) {
+                return true;
+            }
+        }
+
+        return false;
+    }
+
     /**
      * Join two paths, making sure they use the correct directory separators.
      */

src/Parse/Assetic/Filter/HasAllowedImportRoots.php

@@ -0,0 +1,33 @@
+<?php namespace Winter\Storm\Parse\Assetic\Filter;
+
+/**
+ * Shared storage for the additional import roots an asset-combiner filter is willing
+ * to resolve imports into, beyond the source file's own directory subtree.
+ *
+ * Configured by the caller (typically `System\Classes\CombineAssets`) to permit
+ * legitimate cross-tree imports — e.g. a plugin asset importing a module asset — while
+ * still confining everything else. The stored roots are enforced via
+ * {@see \Winter\Storm\Filesystem\PathResolver::withinAny()}.
+ */
+trait HasAllowedImportRoots
+{
+    /**
+     * Additional roots beyond the asset's own source directory that import directives
+     * are allowed to resolve into. Defaults to none — the asset's own directory subtree
+     * is always allowed implicitly by the confinement check.
+     *
+     * @var string[]
+     */
+    protected array $allowedImportRoots = [];
+
+    /**
+     * Configure additional roots that import directives may resolve into. The source
+     * file's own directory is always allowed; this list adds cross-tree destinations.
+     *
+     * @param string[] $roots
+     */
+    public function setAllowedImportRoots(array $roots): void
+    {
+        $this->allowedImportRoots = $roots;
+    }
+}

src/Parse/Assetic/Filter/JavascriptImporter.php

@@ -3,6 +3,7 @@
 use RuntimeException;
 use Assetic\Filter\BaseFilter;
 use Assetic\Contracts\Asset\AssetInterface;
+use Winter\Storm\Filesystem\PathResolver;
 use Winter\Storm\Support\Facades\File;
 
 /**
@@ -20,6 +21,8 @@
  */
 class JavascriptImporter extends BaseFilter
 {
+    use HasAllowedImportRoots;
+
     /**
      * @var string Location of where the processed JS script resides.
      */
@@ -92,13 +95,39 @@ protected function directiveInclude($data, $required = false)
         $require = explode(',', $data);
         $result = "";
 
+        /*
+         * The set of roots an include may resolve into: the including file's own
+         * directory subtree plus any caller-configured cross-tree roots. Computed
+         * once here rather than per-iteration; $this->scriptPath is stable across
+         * the loop (nested parsing saves and restores it below).
+         */
+        $allowedRoots = array_merge([$this->scriptPath], $this->allowedImportRoots);
+
         foreach ($require as $script) {
             $script = trim($script);
 
             if (!File::extension($script)) {
                 $script = $script . '.js';
             }
 
+            /*
+             * Only JavaScript files may be inlined. Rejecting any other extension
+             * (e.g. `.env`, `.php`, `.log`) neutralises disclosure of non-JS server
+             * files even before the path-confinement check below, and mirrors the
+             * combiner's CSS importer, which is restricted to `.css`. Extension-less
+             * includes keep working — they are resolved to `.js` immediately above,
+             * before this check. See GHSA-2223-f22x-24cq.
+             */
+            if (strtolower((string) File::extension($script)) !== 'js') {
+                $errorMsg = sprintf("File '%s' has a disallowed extension. in %s", $script, $this->scriptFile);
+                if ($required) {
+                    throw new RuntimeException($errorMsg);
+                }
+
+                $result .= '/* ' . $errorMsg . ' */' . PHP_EOL;
+                continue;
+            }
+
             $scriptPath = realpath($this->scriptPath . '/' . $script);
             if (!File::isFile($scriptPath)) {
                 $errorMsg = sprintf("File '%s' not found. in %s", $script, $this->scriptFile);
@@ -110,6 +139,23 @@ protected function directiveInclude($data, $required = false)
                 continue;
             }
 
+            /*
+             * Confine the resolved path to the including file's own directory subtree
+             * or a caller-configured allowed root, mirroring the LESS importer gate.
+             * Without this, `=include ../../../.env` would resolve via `realpath()`
+             * traversal and inline an arbitrary server-readable file into public
+             * combiner output. See GHSA-2223-f22x-24cq.
+             */
+            if (!PathResolver::withinAny($scriptPath, $allowedRoots)) {
+                $errorMsg = sprintf("File '%s' is outside the allowed import paths. in %s", $script, $this->scriptFile);
+                if ($required) {
+                    throw new RuntimeException($errorMsg);
+                }
+
+                $result .= '/* ' . $errorMsg . ' */' . PHP_EOL;
+                continue;
+            }
+
             /*
              * Exclude duplicates
              */

src/Parse/Assetic/Filter/LessCompiler.php

@@ -18,39 +18,17 @@
  */
 class LessCompiler extends BaseFilter implements HashableInterface, DependencyExtractorInterface
 {
+    use HasAllowedImportRoots;
+
     protected $presets = [];
 
     protected $lastHash;
 
-    /**
-     * Additional roots beyond the asset's own source directory that `@import`
-     * directives are allowed to resolve into. Configured by the caller (typically
-     * `System\Classes\CombineAssets`) to permit legitimate cross-tree imports
-     * (e.g. a plugin asset importing a module asset). Defaults to none — the
-     * asset's own directory subtree is always allowed implicitly via the
-     * resolver's contextDir rule.
-     *
-     * @var string[]
-     */
-    protected array $allowedImportRoots = [];
-
     public function setPresets(array $presets)
     {
         $this->presets = $presets;
     }
 
-    /**
-     * Configure additional roots that `@import` directives may resolve into. The
-     * source file's own directory is always allowed; this list adds cross-tree
-     * destinations.
-     *
-     * @param string[] $roots
-     */
-    public function setAllowedImportRoots(array $roots): void
-    {
-        $this->allowedImportRoots = $roots;
-    }
-
     public function filterLoad(AssetInterface $asset)
     {
         $parser = new Less_Parser();

src/Parse/Assetic/Filter/LessImportResolver.php

@@ -92,16 +92,10 @@ public static function makeResolver(array $allowedRoots, ?string $contextDir = n
                 return $sentinel;
             }
 
-            if ($contextDir !== null && PathResolver::within($resolved, $contextDir)) {
+            if (PathResolver::withinAny($resolved, array_merge([$contextDir], $allowedRoots))) {
                 return [$resolved, dirname($filename)];
             }
 
-            foreach ($allowedRoots as $root) {
-                if (PathResolver::within($resolved, $root)) {
-                    return [$resolved, dirname($filename)];
-                }
-            }
-
             return $sentinel;
         };
     }

tests/Filesystem/PathResolverTest.php

@@ -383,6 +383,45 @@ public function testWithinDirectoryRejectsPrefixCollision()
         }
     }
 
+    /**
+     * {@see PathResolver::withinAny()} returns true if the path is within any of the
+     * given directories. Null, empty and non-string entries are ignored, so callers
+     * can pass an optional (nullable) context directory alongside a roots list
+     * without pre-filtering — the behaviour the JS/LESS importers rely on.
+     */
+    public function testWithinAny()
+    {
+        $base = sys_get_temp_dir() . '/pathresolver-withinany-' . bin2hex(random_bytes(4));
+        mkdir($base . '/allowed/inside', 0777, true);
+        mkdir($base . '/other', 0777, true);
+        mkdir($base . '/outside', 0777, true);
+        file_put_contents($base . '/allowed/inside/file', 'x');
+
+        try {
+            $target = $base . '/allowed/inside/file';
+
+            // Matches when the containing directory appears anywhere in the list.
+            $this->assertTrue(PathResolver::withinAny($target, [$base . '/allowed']));
+            $this->assertTrue(PathResolver::withinAny($target, [$base . '/other', $base . '/allowed']));
+
+            // No directory in the list contains the path.
+            $this->assertFalse(PathResolver::withinAny($target, [$base . '/other', $base . '/outside']));
+
+            // Empty list never matches.
+            $this->assertFalse(PathResolver::withinAny($target, []));
+
+            // Null, empty-string and non-string entries are skipped; a valid later
+            // entry still matches (the nullable context-dir usage in the importers).
+            $this->assertTrue(PathResolver::withinAny($target, [null, '', $base . '/allowed']));
+            $this->assertFalse(PathResolver::withinAny($target, [null, '', $base . '/outside']));
+
+            // A list of only skippable entries never matches.
+            $this->assertFalse(PathResolver::withinAny($target, [null, '', false]));
+        } finally {
+            (new \Winter\Storm\Filesystem\Filesystem())->deleteDirectory($base);
+        }
+    }
+
     public function testWithOpenBaseDirRestrictions()
     {
         $this->markTestSkipped(