Contain @import file loads within base_dir (arbitrary local file read)

lib/css_parser/parser.rb

@@ -173,6 +173,21 @@ def add_block!(block, options = {})
             load_uri!(import_uri, import_options)
           elsif options[:base_dir]
             import_options[:base_dir] = options[:base_dir]
+            # The import path comes from (potentially untrusted) CSS
+            # content, not from the caller. Flag this load so load_file!
+            # keeps it contained within the import root and refuses
+            # absolute paths / `..` traversal that escape it -- otherwise
+            # @import turns into arbitrary local file disclosure. This is
+            # the base_dir sibling of the file://-via-base_uri hole closed
+            # in GHSA-9pmc-p236-855h, which left this arm untouched.
+            #
+            # import_root is the top-level base_dir, threaded through
+            # nested imports so containment is always checked against it.
+            # That still allows legitimate `../` imports that stay inside
+            # the tree (e.g. subdir/import2.css -> ../simple.css) while
+            # rejecting anything that leaves it.
+            import_options[:from_import] = true
+            import_options[:import_root] = options[:import_root] || options[:base_dir]
             load_file!(import_path, import_options)
           end
         end
@@ -558,8 +573,18 @@ def load_uri!(uri, options = {}, deprecated = nil)
     end
 
     # Load a local CSS file.
+    #
+    # +from_import+ (internal) marks a load whose +file_name+ originates
+    # from an @import rule in parsed CSS rather than from the caller. Such
+    # a path is attacker-influenced, so it is contained within the import
+    # root (+import_root+, the top-level +base_dir+): absolute paths and
+    # `..` traversal that escape the root are refused (raising
+    # RemoteFileError when io_exceptions are on, otherwise a silent no-op).
+    # Direct callers (the default, +from_import: false+) are unaffected --
+    # load_file! remains the explicit, trusted local-file API and may load
+    # any readable path.
     def load_file!(file_name, options = {}, deprecated = nil)
-      opts = {base_dir: nil, media_types: :all}
+      opts = {base_dir: nil, media_types: :all, from_import: false, import_root: nil}
 
       if options.is_a? Hash
         opts.merge!(options)
@@ -570,7 +595,29 @@ def load_file!(file_name, options = {}, deprecated = nil)
         opts[:media_types] = deprecated if deprecated
       end
 
+      # Pin the containment root to the first base_dir seen in the chain.
+      # load_file! resets base_dir to each file's own directory as it
+      # recurses, so without this a file loaded from a subdirectory would
+      # shrink the sandbox and reject a legitimate `..` back toward the
+      # caller's original base_dir.
+      opts[:import_root] ||= opts[:base_dir]
+
       file_name = File.expand_path(file_name, opts[:base_dir])
+
+      # Containment check for @import-sourced paths: the resolved file must
+      # stay inside the import root. Rejects both `@import "/etc/passwd"`
+      # (absolute) and `@import "../../secret"` (traversal escaping the
+      # tree) from untrusted CSS, while leaving direct load_file! callers
+      # untouched and still permitting `..` that stays within the root.
+      if opts[:from_import] && (opts[:import_root] || opts[:base_dir])
+        root = File.expand_path((opts[:import_root] || opts[:base_dir]).to_s)
+        unless file_name == root || file_name.start_with?(root + File::SEPARATOR)
+          raise RemoteFileError, file_name if @options[:io_exceptions]
+
+          return
+        end
+      end
+
       return unless File.readable?(file_name)
       return unless circular_reference_check(file_name)
 

test/test_css_parser_import_lfi.rb

@@ -0,0 +1,130 @@
+# frozen_string_literal: true
+
+require_relative 'test_helper'
+require 'tmpdir'
+require 'fileutils'
+
+# Tests for the @import local-file-disclosure remediation in
+# `CssParser::Parser#load_file!` / the `base_dir` arm of `#add_block!`.
+#
+# This is the `base_dir` sibling of the SSRF / file:// hole fixed in
+# GHSA-9pmc-p236-855h. That fix only closed the `file://`-via-`base_uri`
+# arm of the @import handler (see test_css_parser_ssrf.rb); the `base_dir`
+# arm routed the attacker-controlled @import path straight into
+# `File.expand_path` + `File.read` with no containment.
+#
+# Pre-fix behaviour (with `:base_dir` set and no `:base_uri`, import on):
+#   - `@import "/etc/passwd"` reads an absolute path outside base_dir.
+#   - `@import "../../secret"` traverses out of base_dir.
+#   The imported content is merged into the ruleset and surfaced to the
+#   consumer (e.g. inlined by Premailer for local-file input).
+#
+# Post-fix expectations:
+#   1. An @import whose resolved path escapes the import root -- by
+#      absolute path or `..` traversal -- is refused: RemoteFileError
+#      when io_exceptions are on (the default), a silent no-op otherwise.
+#      No File.read of the out-of-root path happens.
+#   2. Legitimate imports that stay inside the root still load, including
+#      `..` segments that remain within the tree (subdir/x -> ../y).
+#   3. `load_file!` called directly by the caller is unchanged: it is the
+#      explicit, trusted local-file API and may load any readable path,
+#      including one outside any base_dir. Only @import-sourced paths are
+#      contained.
+class CssParserImportLfiTests < Minitest::Test
+  include CssParser
+
+  def setup
+    @cp = Parser.new # defaults: import:true, io_exceptions:true, allow_file_uris:false
+  end
+
+  # Build a disposable tree:
+  #   <root>/main.css          (within root)
+  #   <root>/sub/inner.css     (within root, nested)
+  #   <outside>/secret.css     (sibling dir, OUTSIDE root)
+  # Yields [root_dir, secret_path].
+  def with_tree
+    Dir.mktmpdir do |tmp|
+      root = File.join(tmp, 'root')
+      sub = File.join(root, 'sub')
+      FileUtils.mkdir_p(sub)
+      File.write(File.join(root, 'main.css'), 'div { color: lime; }')
+      File.write(File.join(sub, 'inner.css'), 'a { text-decoration: none; }')
+      secret = File.join(tmp, 'secret.css')
+      File.write(secret, '.secret { content: "TOP_SECRET"; }')
+      yield root, secret
+    end
+  end
+
+  # ---- (1) Escaping imports refused ----
+
+  def test_at_import_absolute_path_via_base_dir_is_refused
+    with_tree do |root, secret|
+      File.expects(:read).never # no out-of-root read may happen
+      err = assert_raises(CssParser::RemoteFileError) do
+        @cp.add_block!(%(@import "#{secret}";), base_dir: root)
+      end
+      assert_includes err.message, secret
+      refute_includes @cp.to_s, 'TOP_SECRET'
+    end
+  end
+
+  def test_at_import_traversal_escaping_root_is_refused
+    with_tree do |root, _secret|
+      File.expects(:read).never
+      assert_raises(CssParser::RemoteFileError) do
+        @cp.add_block!(%(@import "../secret.css";), base_dir: root)
+      end
+      refute_includes @cp.to_s, 'TOP_SECRET'
+    end
+  end
+
+  def test_escaping_import_without_io_exceptions_is_silent_noop
+    with_tree do |root, secret|
+      cp = Parser.new(io_exceptions: false)
+      cp.add_block!(%(@import "#{secret}";), base_dir: root) # must not raise
+      refute_includes cp.to_s, 'TOP_SECRET'
+      refute_includes cp.loaded_uris.join(' '), 'secret.css'
+    end
+  end
+
+  # ---- (2) Legitimate in-root imports still load ----
+
+  def test_import_within_base_dir_still_loads
+    with_tree do |root, _secret|
+      @cp.load_file!('main.css', base_dir: root)
+      assert_equal 'color: lime;', @cp.find_by_selector('div').join(' ')
+    end
+  end
+
+  def test_import_with_dotdot_that_stays_within_root_still_loads
+    # sub/inner.css lives in <root>/sub; an @import "../main.css" from it
+    # resolves to <root>/main.css -- a `..` that stays inside the root and
+    # must keep working (mirrors the fixtures' subdir/import2 -> ../simple).
+    with_tree do |root, _secret|
+      File.write(File.join(root, 'sub', 'inner.css'),
+        %(@import "../main.css";\na { text-decoration: none; }))
+      @cp.load_file!('sub/inner.css', base_dir: root)
+      assert_equal 'text-decoration: none;', @cp.find_by_selector('a').join(' ')
+      assert_equal 'color: lime;', @cp.find_by_selector('div').join(' ') # from ../main.css
+    end
+  end
+
+  # ---- (3) Direct load_file! (trusted caller API) is not contained ----
+
+  def test_direct_load_file_outside_base_dir_still_works
+    # Calling load_file! directly with a path outside base_dir is a
+    # deliberate caller action, not an attacker-influenced @import, and
+    # must keep working.
+    with_tree do |root, secret|
+      @cp.load_file!(secret, base_dir: root) # secret is outside root
+      assert_equal 'content: "TOP_SECRET";', @cp.find_by_selector('.secret').join(' ')
+    end
+  end
+
+  def test_direct_load_file_absolute_path_still_works
+    with_tree do |_root, secret|
+      @cp.load_file!(secret)
+      assert_equal 'content: "TOP_SECRET";', @cp.find_by_selector('.secret').join(' ')
+    end
+  end
+end