ruby-changes:50308
From: hsbt <ko1@a...>
Date: Fri, 16 Feb 2018 17:08:13 +0900 (JST)
Subject: [ruby-changes:50308] hsbt:r62422 (trunk): Merge RubyGems 2.7.6 from upstream.
hsbt 2018-02-16 17:08:06 +0900 (Fri, 16 Feb 2018) New Revision: 62422 https://svn.ruby-lang.org/cgi-bin/viewvc.cgi?view=revision&revision=62422 Log: Merge RubyGems 2.7.6 from upstream. It fixed some security vulnerabilities. http://blog.rubygems.org/2018/02/15/2.7.6-released.html Modified files: trunk/lib/rubygems/commands/owner_command.rb trunk/lib/rubygems/commands/setup_command.rb trunk/lib/rubygems/package/tar_header.rb trunk/lib/rubygems/package/tar_writer.rb trunk/lib/rubygems/package.rb trunk/lib/rubygems/server.rb trunk/lib/rubygems/specification.rb trunk/lib/rubygems.rb trunk/test/rubygems/test_gem_commands_owner_command.rb trunk/test/rubygems/test_gem_commands_setup_command.rb trunk/test/rubygems/test_gem_package.rb trunk/test/rubygems/test_gem_package_tar_header.rb trunk/test/rubygems/test_gem_server.rb trunk/test/rubygems/test_gem_specification.rb trunk/test/rubygems/test_gem_util.rb Index: lib/rubygems/specification.rb =================================================================== --- lib/rubygems/specification.rb (revision 62421) +++ lib/rubygems/specification.rb (revision 62422) @@ -15,6 +15,7 @@ require 'rubygems/basic_specification' https://github.com/ruby/ruby/blob/trunk/lib/rubygems/specification.rb#L15 require 'rubygems/stub_specification' require 'rubygems/util/list' require 'stringio' +require 'uri' ## # The Specification class contains the information for a Gem. Typically @@ -2822,10 +2823,16 @@ http://spdx.org/licenses or '#{Gem::Lice https://github.com/ruby/ruby/blob/trunk/lib/rubygems/specification.rb#L2823 raise Gem::InvalidSpecificationException, "#{lazy} is not a summary" end - if homepage and not homepage.empty? and - homepage !~ /\A[a-z][a-z\d+.-]*:/i then - raise Gem::InvalidSpecificationException, - "\"#{homepage}\" is not a URI" + # Make sure a homepage is valid HTTP/HTTPS URI + if homepage and not homepage.empty? + begin + homepage_uri = URI.parse(homepage) + unless [URI::HTTP, URI::HTTPS].member? homepage_uri.class + raise Gem::InvalidSpecificationException, "\"#{homepage}\" is not a valid HTTP URI" + end + rescue URI::InvalidURIError + raise Gem::InvalidSpecificationException, "\"#{homepage}\" is not a valid HTTP URI" + end end # Warnings Index: lib/rubygems/package/tar_header.rb =================================================================== --- lib/rubygems/package/tar_header.rb (revision 62421) +++ lib/rubygems/package/tar_header.rb (revision 62422) @@ -104,25 +104,30 @@ class Gem::Package::TarHeader https://github.com/ruby/ruby/blob/trunk/lib/rubygems/package/tar_header.rb#L104 fields = header.unpack UNPACK_FORMAT new :name => fields.shift, - :mode => fields.shift.oct, - :uid => fields.shift.oct, - :gid => fields.shift.oct, - :size => fields.shift.oct, - :mtime => fields.shift.oct, - :checksum => fields.shift.oct, + :mode => strict_oct(fields.shift), + :uid => strict_oct(fields.shift), + :gid => strict_oct(fields.shift), + :size => strict_oct(fields.shift), + :mtime => strict_oct(fields.shift), + :checksum => strict_oct(fields.shift), :typeflag => fields.shift, :linkname => fields.shift, :magic => fields.shift, - :version => fields.shift.oct, + :version => strict_oct(fields.shift), :uname => fields.shift, :gname => fields.shift, - :devmajor => fields.shift.oct, - :devminor => fields.shift.oct, + :devmajor => strict_oct(fields.shift), + :devminor => strict_oct(fields.shift), :prefix => fields.shift, :empty => empty end + def self.strict_oct(str) + return str.oct if str =~ /\A[0-7]*\z/ + raise ArgumentError, "#{str.inspect} is not an octal string" + end + ## # Creates a new TarHeader using +vals+ Index: lib/rubygems/package/tar_writer.rb =================================================================== --- lib/rubygems/package/tar_writer.rb (revision 62421) +++ lib/rubygems/package/tar_writer.rb (revision 62422) @@ -196,6 +196,8 @@ class Gem::Package::TarWriter https://github.com/ruby/ruby/blob/trunk/lib/rubygems/package/tar_writer.rb#L196 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 Index: lib/rubygems/commands/owner_command.rb =================================================================== --- lib/rubygems/commands/owner_command.rb (revision 62421) +++ lib/rubygems/commands/owner_command.rb (revision 62422) @@ -64,7 +64,7 @@ permission to. https://github.com/ruby/ruby/blob/trunk/lib/rubygems/commands/owner_command.rb#L64 end with_response response do |resp| - owners = YAML.load resp.body + owners = Gem::SafeYAML.load resp.body say "Owners for gem: #{name}" owners.each do |owner| Index: lib/rubygems/commands/setup_command.rb =================================================================== --- lib/rubygems/commands/setup_command.rb (revision 62421) +++ lib/rubygems/commands/setup_command.rb (revision 62422) @@ -351,7 +351,7 @@ By default, this RubyGems will install g https://github.com/ruby/ruby/blob/trunk/lib/rubygems/commands/setup_command.rb#L351 return unless Gem::USE_BUNDLER_FOR_GEMDEPS specs_dir = Gem::Specification.default_specifications_dir - File.join(options[:destdir], specs_dir) unless Gem.win_platform? + specs_dir = File.join(options[:destdir], specs_dir) unless Gem.win_platform? mkdir_p specs_dir # Workaround for non-git environment. @@ -386,7 +386,7 @@ By default, this RubyGems will install g https://github.com/ruby/ruby/blob/trunk/lib/rubygems/commands/setup_command.rb#L386 end bundler_bin_dir = File.join(Gem.default_dir, 'gems', bundler_spec.full_name, bundler_spec.bindir) - File.join(options[:destdir], bundler_bin_dir) unless Gem.win_platform? + bundler_bin_dir = File.join(options[:destdir], bundler_bin_dir) unless Gem.win_platform? mkdir_p bundler_bin_dir bundler_spec.executables.each do |e| cp File.join("bundler", bundler_spec.bindir, e), File.join(bundler_bin_dir, e) Index: lib/rubygems/server.rb =================================================================== --- lib/rubygems/server.rb (revision 62421) +++ lib/rubygems/server.rb (revision 62422) @@ -623,6 +623,18 @@ div.method-source-code pre { color: #ffd https://github.com/ruby/ruby/blob/trunk/lib/rubygems/server.rb#L623 executables = nil if executables.empty? executables.last["is_last"] = true if executables + # Pre-process spec homepage for safety reasons + begin + homepage_uri = URI.parse(spec.homepage) + if [URI::HTTP, URI::HTTPS].member? homepage_uri.class + homepage_uri = spec.homepage + else + homepage_uri = "." + end + rescue URI::InvalidURIError + homepage_uri = "." + end + specs << { "authors" => spec.authors.sort.join(", "), "date" => spec.date.to_s, @@ -632,7 +644,7 @@ div.method-source-code pre { color: #ffd https://github.com/ruby/ruby/blob/trunk/lib/rubygems/server.rb#L644 "only_one_executable" => (executables && executables.size == 1), "full_name" => spec.full_name, "has_deps" => !deps.empty?, - "homepage" => spec.homepage, + "homepage" => homepage_uri, "name" => spec.name, "rdoc_installed" => Gem::RDoc.new(spec).rdoc_installed?, "ri_installed" => Gem::RDoc.new(spec).ri_installed?, Index: lib/rubygems/package.rb =================================================================== --- lib/rubygems/package.rb (revision 62421) +++ lib/rubygems/package.rb (revision 62422) @@ -378,7 +378,7 @@ EOM https://github.com/ruby/ruby/blob/trunk/lib/rubygems/package.rb#L378 File.dirname destination end - FileUtils.mkdir_p mkdir, mkdir_options + mkdir_p_safe mkdir, mkdir_options, destination_dir, entry.full_name File.open destination, 'wb' do |out| out.write entry.read @@ -416,20 +416,35 @@ EOM https://github.com/ruby/ruby/blob/trunk/lib/rubygems/package.rb#L416 raise Gem::Package::PathError.new(filename, destination_dir) if filename.start_with? '/' - destination_dir = File.realpath destination_dir if - File.respond_to? :realpath + destination_dir = realpath destination_dir destination_dir = File.expand_path destination_dir destination = File.join destination_dir, filename destination = File.expand_path destination raise Gem::Package::PathError.new(destination, destination_dir) unless - destination.start_with? destination_dir + destination.start_with? destination_dir + '/' destination.untaint destination end + def mkdir_p_safe mkdir, mkdir_options, destination_dir, file_name + destination_dir = realpath File.expand_path(destination_dir) + parts = mkdir.split(File::SEPARATOR) + parts.reduce do |path, basename| + path = realpath path unless path == "" + path = File.expand_path(path + File::SEPARATOR + basename) + lstat = File.lstat path rescue nil + if !lstat || !lstat.directory? + unless path.start_with? destination_dir and (FileUtils.mkdir path, mkdir_options rescue false) + raise Gem::Package::PathError.new(file_name, destination_dir) + end + end + path + end + end + ## # Loads a Gem::Specification from the TarEntry +entry+ @@ -603,6 +618,10 @@ EOM https://github.com/ruby/ruby/blob/trunk/lib/rubygems/package.rb#L618 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 ## @@ -616,6 +635,16 @@ EOM https://github.com/ruby/ruby/blob/trunk/lib/rubygems/package.rb#L635 raise Gem::Package::FormatError.new(e.message, entry.full_name) end + if File.respond_to? :realpath + def realpath file + File.realpath file + end + else + def realpath file + file + end + end + end require 'rubygems/package/digest_io' Index: lib/rubygems.rb =================================================================== --- lib/rubygems.rb (revision 62421) +++ lib/rubygems.rb (revision 62422) @@ -10,7 +10,7 @@ require 'rbconfig' https://github.com/ruby/ruby/blob/trunk/lib/rubygems.rb#L10 require 'thread' module Gem - VERSION = "2.7.5" + VERSION = "2.7.6" end # Must be first since it unloads the prelude from 1.9.2 Index: test/rubygems/test_gem_package.rb =================================================================== --- test/rubygems/test_gem_package.rb (revision 62421) +++ test/rubygems/test_gem_package.rb (revision 62422) @@ -455,6 +455,31 @@ class TestGemPackage < Gem::Package::Tar https://github.com/ruby/ruby/blob/trunk/test/rubygems/test_gem_package.rb#L455 File.read(extracted) end + def test_extract_symlink_parent + skip 'symlink not supported' if Gem.win_platform? + + package = Gem::Package.new @gem + + tgz_io = util_tar_gz do |tar| + tar.mkdir 'lib', 0755 + tar.add_symlink 'lib/link', '../..', 0644 + tar.add_file 'lib/link/outside.txt', 0644 do |io| io.write 'hi' end + end + + # Extract into a subdirectory of @destination; if this test fails it writes + # a file outside destination_subdir, but we want the file to remain inside + # @destination so it will be cleaned up. + destination_subdir = File.join @destination, 'subdir' + FileUtils.mkdir_p destination_subdir + + e = assert_raises Gem::Package::PathError do + package.extract_tar_gz tgz_io, destination_subdir + end + + assert_equal("installing into parent path lib/link/outside.txt of " + + "#{destination_subdir} is not allowed", e.message) + end + def test_extract_tar_gz_directory package = Gem::Package.new @gem @@ -566,6 +591,21 @@ class TestGemPackage < Gem::Package::Tar https://github.com/ruby/ruby/blob/trunk/test/rubygems/test_gem_package.rb#L591 "#{@destination} is not allowed", e.message) end + def test_install_location_suffix + package = Gem::Package.new @gem + + filename = "../#{File.basename(@destination)}suffix.rb" + + e = assert_raises Gem::Package::PathError do + package.install_location filename, @destination + end + + parent = File.expand_path File.join @destination, filename + + assert_equal("installing into parent path #{parent} of " + + "#{@destination} is not allowed", e.message) + end + def test_load_spec entry = StringIO.new Gem.gzip @spec.to_yaml def entry.full_name() 'metadata.gz' end @@ -723,6 +763,32 @@ class TestGemPackage < Gem::Package::Tar https://github.com/ruby/ruby/blob/trunk/test/rubygems/test_gem_package.rb#L763 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) @@ -780,7 +846,13 @@ class TestGemPackage < Gem::Package::Tar https://github.com/ruby/ruby/blob/trunk/test/rubygems/test_gem_package.rb#L846 # 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 Index: test/rubygems/test_gem_commands_owner_command.rb =================================================================== --- test/rubygems/test_gem_commands_owner_command.rb (revision 62421) +++ test/rubygems/test_gem_commands_owner_command.rb (revision 62422) @@ -43,6 +43,31 @@ EOF https://github.com/ruby/ruby/blob/trunk/test/rubygems/test_gem_commands_owner_command.rb#L43 assert_match %r{- 4}, @ui.output end + def test_show_owners_dont_load_objects + skip "testing a psych-only API" unless defined?(::Psych::DisallowedClass) + + response = <<EOF +--- +- email: !ruby/object:Object {} + id: 1 + handle: user1 +- email: user2@e... +- id: 3 + handle: user3 +- id: 4 +EOF + + @fetcher.data["#{Gem.host}/api/v1/gems/freewill/owners.yaml"] = [response, 200, 'OK'] + + assert_raises Psych::DisallowedClass do + use_ui @ui do + @cmd.show_owners("freewill") + end + end + + end + + def test_show_owners_setting_up_host_through_env_var response = "- email: user1@e...\n" host = "http://rubygems.example" Index: test/rubygems/test_gem_commands_setup_command.rb =================================================================== --- test/rubygems/test_gem_commands_setup_command.rb (revision 62421) +++ test/rubygems/test_gem_commands_setup_command.rb (revision 62422) @@ -6,7 +6,12 @@ require 'rubygems/commands/setup_command https://github.com/ruby/ruby/blob/trunk/test/rubygems/test_gem_commands_setup_command.rb#L6 class TestGemCommandsSetupCommand < Gem::TestCase - BUNDLER_VERS = "1.16.1" + bundler_gemspec = File.expand_path("../../../bundler/lib/bundler/version.rb", __FILE__) + if File.exist?(bundler_gemspec) + BUNDLER_VERS = File.read(bundler_gemspec).match(/VERSION = "(#{Gem::Version::VERSION_PATTERN})"/)[1] + else + BUNDLER_VERS = "1.16.1" + end def setup super Index: test/rubygems/test_gem_specification.rb =================================================================== --- test/rubygems/test_gem_specification.rb (revision 62421) +++ test/rubygems/test_gem_specification.rb (revision 62422) @@ -2886,7 +2886,22 @@ duplicate dependency on c (>= 1.2.3, dev https://github.com/ruby/ruby/blob/trunk/test/rubygems/test_gem_specification.rb#L2886 @a1.validate end - assert_equal '"over at my cool site" is not a URI', e.message + assert_equal '"over at my cool site" is not a valid HTTP URI', e.message + + @a1.homepage = 'ftp://rubygems.org' + + e = assert_raises Gem::InvalidSpecificationException do + @a1.validate + end + + assert_equal '"ftp://rubygems.org" is not a valid HTTP URI', e.message + + @a1.homepage = 'http://rubygems.org' + assert_equal true, @a1.validate + + @a1.homepage = 'https://rubygems.org' + assert_equal true, @a1.validate + end end Index: test/rubygems/test_gem_package_tar_header.rb =================================================================== --- test/rubygems/test_gem_package_tar_header.rb (revision 62421) +++ test/rubygems/test_gem_package_tar_header.rb (revision 62422) @@ -143,5 +143,25 @@ group\000\000\000\000\000\000\000\000\00 https://github.com/ruby/ruby/blob/trunk/test/rubygems/test_gem_package_tar_header.rb#L143 assert_equal '012467', @tar_header.checksum end + def test_from_bad_octal + test_cases = [ + "00000006,44\000", # bogus character + "00000006789\000", # non-octal digit + "+0000001234\000", # positive sign + "-0000001000\000", # negative sign + "0x000123abc\000", # radix prefix + ] + + test_cases.each do |val| + header_s = @tar_header.to_s + # overwrite the size field + header_s[124, 12] = val + io = TempIO.new header_s + assert_raises ArgumentError do + new_header = Gem::Package::TarHeader.from io + end + end + end + end Index: test/rubygems/test_gem_util.rb =================================================================== --- test/rubygems/test_gem_util.rb (revision 62421) +++ test/rubygems/test_gem_util.rb (revision 62422) @@ -42,8 +42,13 @@ class TestGemUtil < Gem::TestCase https://github.com/ruby/ruby/blob/trunk/test/rubygems/test_gem_util.rb#L42 assert_equal File.join(@tempdir, 'd'), paths[0] assert_equal @tempdir, paths[1] - assert_equal File.realpath(Dir.tmpdir), paths[2] - assert_equal File.realpath("..", Dir.tmpdir), paths[3] + if File.respond_to?(:realpath) + assert_equal File.realpath(Dir.tmpdir), paths[2] + assert_equal File.realpath("..", Dir.tmpdir), paths[3] + elsif RUBY_PLATFORM !~ /darwin/ + assert_equal Dir.tmpdir, paths[2] + assert_equal '/', paths[3] + end ensure # restore default permissions, allow the directory to be removed FileUtils.chmod(0775, 'd/e') unless win_platform? Index: test/rubygems/test_gem_server.rb =================================================================== --- test/rubygems/test_gem_server.rb (revision 62421) +++ test/rubygems/test_gem_server.rb (revision 62422) @@ -353,6 +353,171 @@ class TestGemServer < Gem::TestCase https://github.com/ruby/ruby/blob/trunk/test/rubygems/test_gem_server.rb#L353 assert_match 'z 9', @res.body end + + def test_xss_homepage_fix_289313 + data = StringIO.new "GET / HTTP/1.0\r\n\r\n" + dir = "#{@gemhome}2" + + spec = util_spec 'xsshomepagegem', 1 + spec.homepage = "javascript:confirm(document.domain)" + + specs_dir = File.join dir, 'specifications' + FileUtils.mkdir_p specs_dir + + open File.join(specs_dir, spec.spec_name), 'w' do |io| + io.write spec.to_ruby + end + + server = Gem::Server.new dir, process_based_port, false + + @req.parse data + + server.root @req, @res + + assert_equal 200, @res.status + assert_match 'xsshomepagegem 1', @res.body + + # This verifies that the homepage for this spec is not displayed and is set to ".", because it's not a + # valid HTTP/HTTPS URL and could be un (... truncated) -- ML: ruby-changes@q... Info: http://www.atdot.net/~ko1/quickml/