Skip to content

Commit 4b1c454

Browse files
hsbtclaude
andcommitted
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 56ed326, and apply the same guard to the independent Gem::CompactIndexClient::Cache port. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent ceed015 commit 4b1c454

5 files changed

Lines changed: 65 additions & 0 deletions

File tree

lib/bundler/compact_index_client/cache.rb

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ def versions_etag_path = directory.join("versions.etag")
5050

5151
def info_path(name)
5252
name = name.to_s
53+
validate_name!(name)
5354
# TODO: converge this into the info_root by hashing all filenames like info_etag_path
5455
if /[^a-z0-9_-]/.match?(name)
5556
name += "-#{SharedHelpers.digest(:MD5).hexdigest(name).downcase}"
@@ -61,9 +62,19 @@ def info_path(name)
6162

6263
def info_etag_path(name)
6364
name = name.to_s
65+
validate_name!(name)
6466
@info_etag_root.join("#{name}-#{SharedHelpers.digest(:MD5).hexdigest(name).downcase}")
6567
end
6668

69+
# Gem names come from the remote index and are not otherwise
70+
# validated, so refuse anything that would escape the cache
71+
# directory when used as a path component.
72+
def validate_name!(name)
73+
return if File.basename(name) == name
74+
75+
raise SecurityError, "malformed gem name: #{name.inspect}"
76+
end
77+
6778
def mkdir(name)
6879
directory.join(name).tap do |dir|
6980
SharedHelpers.filesystem_access(dir) do

lib/rubygems/compact_index_client/cache.rb

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@ def versions_etag_path = directory.join("versions.etag")
6060

6161
def info_path(name)
6262
name = name.to_s
63+
validate_name!(name)
6364
if /[^a-z0-9_-]/.match?(name)
6465
name += "-#{Digest::MD5.hexdigest(name).downcase}"
6566
@special_characters_info_root.join(name)
@@ -70,9 +71,19 @@ def info_path(name)
7071

7172
def info_etag_path(name)
7273
name = name.to_s
74+
validate_name!(name)
7375
@info_etag_root.join("#{name}-#{Digest::MD5.hexdigest(name).downcase}")
7476
end
7577

78+
# Gem names come from the remote index and are not otherwise
79+
# validated, so refuse anything that would escape the cache
80+
# directory when used as a path component.
81+
def validate_name!(name)
82+
return if File.basename(name) == name
83+
84+
raise Gem::Exception, "malformed gem name: #{name.inspect}"
85+
end
86+
7687
def checksum_for_file(path)
7788
return unless path.file?
7889
Digest::MD5.file(path).hexdigest
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
# frozen_string_literal: true
2+
3+
require "bundler/compact_index_client"
4+
require "bundler/compact_index_client/cache"
5+
require "tmpdir"
6+
7+
RSpec.describe Bundler::CompactIndexClient::Cache do
8+
let(:directory) { Pathname.new(Dir.mktmpdir("compact_index")) }
9+
let(:cache) { described_class.new(directory) }
10+
11+
it "rejects a gem name that would escape the cache directory" do
12+
expect { cache.info("../../../../pwn") }.to raise_error(Bundler::SecurityError, /malformed gem name/)
13+
end
14+
15+
it "accepts plain gem names, including special characters" do
16+
expect { cache.info("rack") }.not_to raise_error
17+
expect { cache.info("Rails.js") }.not_to raise_error
18+
end
19+
end

spec/support/shards.rb

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -148,6 +148,7 @@ module Shards
148148
],
149149
shard_d: [
150150
"spec/bundler/compact_index_client/cache_file_spec.rb",
151+
"spec/bundler/compact_index_client/cache_spec.rb",
151152
"spec/bundler/rubygems_ext_spec.rb",
152153
"spec/bundler/resolver/cooldown_spec.rb",
153154
"spec/install/cooldown_spec.rb",

test/rubygems/test_gem_compact_index_client_cache.rb

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,29 @@ def test_fetch_info_fetches_without_checksum
113113
assert_equal "a 1.0.0\n", @dir.join("info", "a").read
114114
end
115115

116+
def test_info_rejects_name_escaping_cache_directory
117+
fetcher = FakeFetcher.new("1.0.0\n")
118+
cache = Gem::CompactIndexClient::Cache.new(@dir, fetcher)
119+
120+
e = assert_raise Gem::Exception do
121+
cache.info("../../../../pwn", "no-match")
122+
end
123+
124+
assert_includes e.message, "malformed gem name"
125+
assert_empty fetcher.requests
126+
end
127+
128+
def test_fetch_info_rejects_name_escaping_cache_directory
129+
fetcher = FakeFetcher.new("1.0.0\n")
130+
cache = Gem::CompactIndexClient::Cache.new(@dir, fetcher)
131+
132+
assert_raise Gem::Exception do
133+
cache.fetch_info("../pwn")
134+
end
135+
136+
assert_empty fetcher.requests
137+
end
138+
116139
def test_info_with_special_characters_uses_hashed_path
117140
fetcher = FakeFetcher.new("1.0.0\n")
118141
cache = Gem::CompactIndexClient::Cache.new(@dir, fetcher)

0 commit comments

Comments
 (0)