Merge pull request #4651 from nobu/check-requirements-classes

Check requirements class before loading marshalled requirements

Manifest.txt

@@ -486,7 +486,6 @@ lib/rubygems/ssl_certs/.document
 lib/rubygems/ssl_certs/rubygems.org/GlobalSignRootCA.pem
 lib/rubygems/ssl_certs/rubygems.org/GlobalSignRootCA_R3.pem
 lib/rubygems/stub_specification.rb
-lib/rubygems/syck_hack.rb
 lib/rubygems/text.rb
 lib/rubygems/uninstaller.rb
 lib/rubygems/unknown_command_spell_checker.rb

lib/rubygems.rb

@@ -628,23 +628,13 @@ def self.load_yaml
     rescue ::LoadError
       # If we can't load psych, that's fine, go on.
     else
-      # If 'yaml' has already been required, then we have to
-      # be sure to switch it over to the newly loaded psych.
-      if defined?(YAML::ENGINE) && YAML::ENGINE.yamler != "psych"
-        YAML::ENGINE.yamler = "psych"
-      end
-
       require 'rubygems/psych_additions'
       require 'rubygems/psych_tree'
     end
 
     require 'yaml'
     require 'rubygems/safe_yaml'
 
-    # Now that we're sure some kind of yaml library is loaded, pull
-    # in our hack to deal with Syck's DefaultKey ugliness.
-    require 'rubygems/syck_hack'
-
     @yaml_loaded = true
   end
 

lib/rubygems/requirement.rb

@@ -194,24 +194,19 @@ def hash # :nodoc:
   end
 
   def marshal_dump # :nodoc:
-    fix_syck_default_key_in_requirements
-
     [@requirements]
   end
 
   def marshal_load(array) # :nodoc:
     @requirements = array[0]
 
-    fix_syck_default_key_in_requirements
+    raise TypeError, "wrong @requirements" unless Array === @requirements
   end
 
   def yaml_initialize(tag, vals) # :nodoc:
     vals.each do |ivar, val|
       instance_variable_set "@#{ivar}", val
     end
-
-    Gem.load_yaml
-    fix_syck_default_key_in_requirements
   end
 
   def init_with(coder) # :nodoc:
@@ -246,8 +241,7 @@ def pretty_print(q) # :nodoc:
   def satisfied_by?(version)
     raise ArgumentError, "Need a Gem::Version: #{version.inspect}" unless
       Gem::Version === version
-    # #28965: syck has a bug with unquoted '=' YAML.loading as YAML::DefaultKey
-    requirements.all? {|op, rv| (OPS[op] || OPS["="]).call version, rv }
+    requirements.all? {|op, rv| OPS[op].call version, rv }
   end
 
   alias :=== :satisfied_by?
@@ -289,19 +283,6 @@ def _sorted_requirements
   def _tilde_requirements
     @_tilde_requirements ||= _sorted_requirements.select {|r| r.first == "~>" }
   end
-
-  private
-
-  def fix_syck_default_key_in_requirements # :nodoc:
-    Gem.load_yaml
-
-    # Fixup the Syck DefaultKey bug
-    @requirements.each do |r|
-      if r[0].kind_of? Gem::SyckDefaultKey
-        r[0] = "="
-      end
-    end
-  end
 end
 
 class Gem::Version

lib/rubygems/safe_yaml.rb

@@ -17,8 +17,6 @@ module SafeYAML
       Gem::Specification
       Gem::Version
       Gem::Version::Requirement
-      YAML::Syck::DefaultKey
-      Syck::DefaultKey
     ].freeze
 
     PERMITTED_SYMBOLS = %w[

lib/rubygems/specification.rb

@@ -1690,12 +1690,6 @@ def date=(date)
             when String then
               if DateTimeFormat =~ date
                 Time.utc($1.to_i, $2.to_i, $3.to_i)
-
-              # Workaround for where the date format output from psych isn't
-              # parsed as a Time object by syck and thus comes through as a
-              # string.
-              elsif /\A(\d{4})-(\d{2})-(\d{2}) \d{2}:\d{2}:\d{2}\.\d+?Z\z/ =~ date
-                Time.utc($1.to_i, $2.to_i, $3.to_i)
               else
                 raise(Gem::InvalidSpecificationException,
                       "invalid date format in specification: #{date.inspect}")

test/rubygems/test_gem_requirement.rb

@@ -422,6 +422,40 @@ def test_hash_returns_equal_hashes_for_equivalent_requirements
     assert_requirement_hash_equal "1", "1.0.0"
   end
 
+  class Exploit < RuntimeError
+  end
+
+  def self.exploit(arg)
+    raise Exploit, "arg = #{arg}"
+  end
+
+  def test_marshal_load_attack
+    wa = Net::WriteAdapter.allocate
+    wa.instance_variable_set(:@socket, self.class)
+    wa.instance_variable_set(:@method_id, :exploit)
+    request_set = Gem::RequestSet.allocate
+    request_set.instance_variable_set(:@git_set, "id")
+    request_set.instance_variable_set(:@sets, wa)
+    wa = Net::WriteAdapter.allocate
+    wa.instance_variable_set(:@socket, request_set)
+    wa.instance_variable_set(:@method_id, :resolve)
+    ent = Gem::Package::TarReader::Entry.allocate
+    ent.instance_variable_set(:@read, 0)
+    ent.instance_variable_set(:@header, "aaa")
+    io = Net::BufferedIO.allocate
+    io.instance_variable_set(:@io, ent)
+    io.instance_variable_set(:@debug_output, wa)
+    reader = Gem::Package::TarReader.allocate
+    reader.instance_variable_set(:@io, io)
+    requirement = Gem::Requirement.allocate
+    requirement.instance_variable_set(:@requirements, reader)
+    m = [Gem::SpecFetcher, Gem::Installer, requirement]
+    e = assert_raise(TypeError) do
+      Marshal.load(Marshal.dump(m))
+    end
+    assert_equal(e.message, "wrong @requirements")
+  end
+
   # Assert that two requirements are equal. Handles Gem::Requirements,
   # strings, arrays, numbers, and versions.