From c74b4590a9bffa7888043483b80be5f301553730 Mon Sep 17 00:00:00 2001
From: David Fifield <david@bamsoftware.com>
Date: Thu, 5 Oct 2017 15:49:39 -0700
Subject: [PATCH] Add tests that Gem::Package::verify checks duplicate
 filenames.

Two new tests check that verify detects an error when the gem tar file
contains multiple non-identical entries for metadata.gz, data.tar.gz,
and checksums.yaml.gz. There is only one set of .sig files.
test_verify_security_policy_bogus_before
	adds the bogus entries in pre-position: before the valid entries
test_verify_security_policy_bogus_after
	adds the bogus entries in post-position: after the valid entries

The existing test test_verify_security_policy_checksum_missing
incidentally tests a duplicate data.tar.gz in post-position, even though
its purpose is to test verify in the absence of checksums. The current
code catches this post-position case, but would not notice if
data.tar.gz were in pre-position.

I put the new tests at MediumSecurity, because signatures are supposed
to be verified at medium and high levels.
---
 test/rubygems/test_gem_package.rb | 110 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 110 insertions(+)

diff --git a/test/rubygems/test_gem_package.rb b/test/rubygems/test_gem_package.rb
index cec1981c..68900828 100644
--- a/test/rubygems/test_gem_package.rb
+++ b/test/rubygems/test_gem_package.rb
@@ -803,6 +803,116 @@ class TestGemPackage < Gem::Package::TarTestCase
     assert_empty package.instance_variable_get(:@files), '@files must empty'
   end
 
+  def test_verify_security_policy_bogus_before
+    skip 'openssl is missing' unless defined?(OpenSSL::SSL)
+
+    @spec.cert_chain = [PUBLIC_CERT.to_pem]
+    @spec.signing_key = PRIVATE_KEY
+
+    build = Gem::Package.new @gem
+    build.spec = @spec
+    build.setup_signer
+
+    FileUtils.mkdir 'lib'
+    FileUtils.touch 'lib/code.rb'
+
+    open @gem, 'wb' do |gem_io|
+      Gem::Package::TarWriter.new gem_io do |gem|
+        # write bogus metadata.gz
+        bogus_data = Gem.gzip 'hello'
+        gem.add_file_simple 'metadata.gz', 0444, bogus_data.length do |io|
+          io.write bogus_data
+        end
+
+        # write bogus data.tar.gz
+        bogus_data = Gem.gzip 'hello'
+        gem.add_file_simple 'data.tar.gz', 0444, bogus_data.length do |io|
+          io.write bogus_data
+        end
+
+        # write bogus checksums.yaml.gz
+        bogus_data = Gem.gzip 'hello'
+        gem.add_file_simple 'checksums.yaml.gz', 0444, bogus_data.length do |io|
+          io.write bogus_data
+        end
+
+        # write the actual signed data
+        build.add_metadata gem
+        build.add_contents gem
+        build.add_checksums gem
+      end
+    end
+
+    Gem::Security.trust_dir.trust_cert PUBLIC_CERT
+
+    package = Gem::Package.new @gem
+    package.security_policy = Gem::Security::MediumSecurity
+
+    e = assert_raises Gem::Security::Exception do
+      package.verify
+    end
+
+    assert_equal 'invalid signature', e.message
+
+    refute package.instance_variable_get(:@spec), '@spec must not be loaded'
+    assert_empty package.instance_variable_get(:@files), '@files must empty'
+  end
+
+  def test_verify_security_policy_bogus_after
+    skip 'openssl is missing' unless defined?(OpenSSL::SSL)
+
+    @spec.cert_chain = [PUBLIC_CERT.to_pem]
+    @spec.signing_key = PRIVATE_KEY
+
+    build = Gem::Package.new @gem
+    build.spec = @spec
+    build.setup_signer
+
+    FileUtils.mkdir 'lib'
+    FileUtils.touch 'lib/code.rb'
+
+    open @gem, 'wb' do |gem_io|
+      Gem::Package::TarWriter.new gem_io do |gem|
+        # write the actual signed data
+        build.add_metadata gem
+        build.add_contents gem
+        build.add_checksums gem
+
+        # write bogus metadata.gz
+        bogus_data = Gem.gzip 'hello'
+        gem.add_file_simple 'metadata.gz', 0444, bogus_data.length do |io|
+          io.write bogus_data
+        end
+
+        # write bogus data.tar.gz
+        bogus_data = Gem.gzip 'hello'
+        gem.add_file_simple 'data.tar.gz', 0444, bogus_data.length do |io|
+          io.write bogus_data
+        end
+
+        # write bogus checksums.yaml.gz
+        bogus_data = Gem.gzip 'hello'
+        gem.add_file_simple 'checksums.yaml.gz', 0444, bogus_data.length do |io|
+          io.write bogus_data
+        end
+      end
+    end
+
+    Gem::Security.trust_dir.trust_cert PUBLIC_CERT
+
+    package = Gem::Package.new @gem
+    package.security_policy = Gem::Security::MediumSecurity
+
+    e = assert_raises Gem::Security::Exception do
+      package.verify
+    end
+
+    assert_equal 'invalid signature', e.message
+
+    refute package.instance_variable_get(:@spec), '@spec must not be loaded'
+    assert_empty package.instance_variable_get(:@files), '@files must empty'
+  end
+
   def test_verify_truncate
     open 'bad.gem', 'wb' do |io|
       io.write File.read(@gem, 1024) # don't care about newlines
-- 
2.14.2