From 4b1c4545260ba1e1eedbe623a71dc10fd2a97f2d Mon Sep 17 00:00:00 2001 From: Hiroshi SHIBATA Date: Tue, 21 Jul 2026 19:23:28 +0900 Subject: [PATCH] Validate gem name before building compact index cache paths Bundler::CompactIndexClient::Cache#info_path and #info_etag_path join the gem name into the cache directory without validation. The name comes from the remote index, either versions lines or transitive dependency names in info files, so a crafted name like "../../../../pwn" escapes the cache directory because the special-characters branch keeps the raw name and only appends an MD5 suffix. Reject any name that is not a plain basename before constructing the path, matching the fetch_spec guard from 56ed326cb2, and apply the same guard to the independent Gem::CompactIndexClient::Cache port. Co-Authored-By: Claude Fable 5 --- lib/bundler/compact_index_client/cache.rb | 11 +++++++++ lib/rubygems/compact_index_client/cache.rb | 11 +++++++++ .../compact_index_client/cache_spec.rb | 19 +++++++++++++++ spec/support/shards.rb | 1 + .../test_gem_compact_index_client_cache.rb | 23 +++++++++++++++++++ 5 files changed, 65 insertions(+) create mode 100644 spec/bundler/compact_index_client/cache_spec.rb diff --git a/lib/bundler/compact_index_client/cache.rb b/lib/bundler/compact_index_client/cache.rb index 3bae6c9efdf8..c47535a01961 100644 --- a/lib/bundler/compact_index_client/cache.rb +++ b/lib/bundler/compact_index_client/cache.rb @@ -50,6 +50,7 @@ def versions_etag_path = directory.join("versions.etag") def info_path(name) name = name.to_s + validate_name!(name) # TODO: converge this into the info_root by hashing all filenames like info_etag_path if /[^a-z0-9_-]/.match?(name) name += "-#{SharedHelpers.digest(:MD5).hexdigest(name).downcase}" @@ -61,9 +62,19 @@ def info_path(name) def info_etag_path(name) name = name.to_s + validate_name!(name) @info_etag_root.join("#{name}-#{SharedHelpers.digest(:MD5).hexdigest(name).downcase}") end + # Gem names come from the remote index and are not otherwise + # validated, so refuse anything that would escape the cache + # directory when used as a path component. + def validate_name!(name) + return if File.basename(name) == name + + raise SecurityError, "malformed gem name: #{name.inspect}" + end + def mkdir(name) directory.join(name).tap do |dir| SharedHelpers.filesystem_access(dir) do diff --git a/lib/rubygems/compact_index_client/cache.rb b/lib/rubygems/compact_index_client/cache.rb index 34e20dfea0b8..84c74fb86a1f 100644 --- a/lib/rubygems/compact_index_client/cache.rb +++ b/lib/rubygems/compact_index_client/cache.rb @@ -60,6 +60,7 @@ def versions_etag_path = directory.join("versions.etag") def info_path(name) name = name.to_s + validate_name!(name) if /[^a-z0-9_-]/.match?(name) name += "-#{Digest::MD5.hexdigest(name).downcase}" @special_characters_info_root.join(name) @@ -70,9 +71,19 @@ def info_path(name) def info_etag_path(name) name = name.to_s + validate_name!(name) @info_etag_root.join("#{name}-#{Digest::MD5.hexdigest(name).downcase}") end + # Gem names come from the remote index and are not otherwise + # validated, so refuse anything that would escape the cache + # directory when used as a path component. + def validate_name!(name) + return if File.basename(name) == name + + raise Gem::Exception, "malformed gem name: #{name.inspect}" + end + def checksum_for_file(path) return unless path.file? Digest::MD5.file(path).hexdigest diff --git a/spec/bundler/compact_index_client/cache_spec.rb b/spec/bundler/compact_index_client/cache_spec.rb new file mode 100644 index 000000000000..85528c0cac32 --- /dev/null +++ b/spec/bundler/compact_index_client/cache_spec.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +require "bundler/compact_index_client" +require "bundler/compact_index_client/cache" +require "tmpdir" + +RSpec.describe Bundler::CompactIndexClient::Cache do + let(:directory) { Pathname.new(Dir.mktmpdir("compact_index")) } + let(:cache) { described_class.new(directory) } + + it "rejects a gem name that would escape the cache directory" do + expect { cache.info("../../../../pwn") }.to raise_error(Bundler::SecurityError, /malformed gem name/) + end + + it "accepts plain gem names, including special characters" do + expect { cache.info("rack") }.not_to raise_error + expect { cache.info("Rails.js") }.not_to raise_error + end +end diff --git a/spec/support/shards.rb b/spec/support/shards.rb index 9fd67f921b4f..24be0e5e9a55 100644 --- a/spec/support/shards.rb +++ b/spec/support/shards.rb @@ -148,6 +148,7 @@ module Shards ], shard_d: [ "spec/bundler/compact_index_client/cache_file_spec.rb", + "spec/bundler/compact_index_client/cache_spec.rb", "spec/bundler/rubygems_ext_spec.rb", "spec/bundler/resolver/cooldown_spec.rb", "spec/install/cooldown_spec.rb", diff --git a/test/rubygems/test_gem_compact_index_client_cache.rb b/test/rubygems/test_gem_compact_index_client_cache.rb index e34ddce6f633..2951f8e32419 100644 --- a/test/rubygems/test_gem_compact_index_client_cache.rb +++ b/test/rubygems/test_gem_compact_index_client_cache.rb @@ -113,6 +113,29 @@ def test_fetch_info_fetches_without_checksum assert_equal "a 1.0.0\n", @dir.join("info", "a").read end + def test_info_rejects_name_escaping_cache_directory + fetcher = FakeFetcher.new("1.0.0\n") + cache = Gem::CompactIndexClient::Cache.new(@dir, fetcher) + + e = assert_raise Gem::Exception do + cache.info("../../../../pwn", "no-match") + end + + assert_includes e.message, "malformed gem name" + assert_empty fetcher.requests + end + + def test_fetch_info_rejects_name_escaping_cache_directory + fetcher = FakeFetcher.new("1.0.0\n") + cache = Gem::CompactIndexClient::Cache.new(@dir, fetcher) + + assert_raise Gem::Exception do + cache.fetch_info("../pwn") + end + + assert_empty fetcher.requests + end + def test_info_with_special_characters_uses_hashed_path fetcher = FakeFetcher.new("1.0.0\n") cache = Gem::CompactIndexClient::Cache.new(@dir, fetcher)