Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions lib/bundler/compact_index_client/cache.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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}"
Expand All @@ -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
Expand Down
11 changes: 11 additions & 0 deletions lib/rubygems/compact_index_client/cache.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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
Expand Down
19 changes: 19 additions & 0 deletions spec/bundler/compact_index_client/cache_spec.rb
Original file line number Diff line number Diff line change
@@ -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
1 change: 1 addition & 0 deletions spec/support/shards.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
23 changes: 23 additions & 0 deletions test/rubygems/test_gem_compact_index_client_cache.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down