Skip to content
Permalink
Browse files Browse the repository at this point in the history
Raise a security error when there are duplicate files in a package
This is a rudimentary fix for an issue where RubyGems would allow a
mis-signed gem to be installed, as the tarball would contain multiple
gem signatures.

Nothing should give us a tarball with multiple entries, so we'll just
disallow that.
  • Loading branch information
segiddins committed Feb 16, 2018
1 parent 21872ae commit f5042b8
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 1 deletion.
4 changes: 4 additions & 0 deletions lib/rubygems/package.rb
Expand Up @@ -603,6 +603,10 @@ def verify_files gem
raise Gem::Package::FormatError.new \
'package content (data.tar.gz) is missing', @gem
end

if duplicates = @files.group_by {|f| f }.select {|k,v| v.size > 1 }.map(&:first) and duplicates.any?
raise Gem::Security::Exception, "duplicate files in the package: (#{duplicates.map(&:inspect).join(', ')})"
end
end

##
Expand Down
2 changes: 2 additions & 0 deletions lib/rubygems/package/tar_writer.rb
Expand Up @@ -196,6 +196,8 @@ def add_file_signed name, mode, signer
digest_name == signer.digest_name
end

raise "no #{signer.digest_name} in #{digests.values.compact}" unless signature_digest

if signer.key then
signature = signer.sign signature_digest.digest

Expand Down
34 changes: 33 additions & 1 deletion test/rubygems/test_gem_package.rb
Expand Up @@ -738,6 +738,32 @@ def test_verify_nonexistent
assert_match %r%nonexistent.gem$%, e.message
end

def test_verify_duplicate_file
FileUtils.mkdir_p 'lib'
FileUtils.touch 'lib/code.rb'

build = Gem::Package.new @gem
build.spec = @spec
build.setup_signer
open @gem, 'wb' do |gem_io|
Gem::Package::TarWriter.new gem_io do |gem|
build.add_metadata gem
build.add_contents gem

gem.add_file_simple 'a.sig', 0444, 0
gem.add_file_simple 'a.sig', 0444, 0
end
end

package = Gem::Package.new @gem

e = assert_raises Gem::Security::Exception do
package.verify
end

assert_equal 'duplicate files in the package: ("a.sig")', e.message
end

def test_verify_security_policy
skip 'openssl is missing' unless defined?(OpenSSL::SSL)

Expand Down Expand Up @@ -795,7 +821,13 @@ def test_verify_security_policy_checksum_missing

# write bogus data.tar.gz to foil signature
bogus_data = Gem.gzip 'hello'
gem.add_file_simple 'data.tar.gz', 0444, bogus_data.length do |io|
fake_signer = Class.new do
def digest_name; 'SHA512'; end
def digest_algorithm; Digest(:SHA512); end
def key; 'key'; end
def sign(*); 'fake_sig'; end
end
gem.add_file_signed 'data2.tar.gz', 0444, fake_signer.new do |io|
io.write bogus_data
end

Expand Down

0 comments on commit f5042b8

Please sign in to comment.