ruby-changes:30960
From: drbrain <ko1@a...>
Date: Wed, 25 Sep 2013 09:53:30 +0900 (JST)
Subject: [ruby-changes:30960] drbrain:r43039 (trunk): * lib/rubygems: Fix CVE-2013-4363. Miscellaneous minor improvements.
drbrain 2013-09-25 09:53:19 +0900 (Wed, 25 Sep 2013) New Revision: 43039 http://svn.ruby-lang.org/cgi-bin/viewvc.cgi?view=rev&revision=43039 Log: * lib/rubygems: Fix CVE-2013-4363. Miscellaneous minor improvements. * test/rubygems: Tests for the above. Modified files: trunk/ChangeLog trunk/lib/rubygems/commands/specification_command.rb trunk/lib/rubygems/commands/unpack_command.rb trunk/lib/rubygems/commands/update_command.rb trunk/lib/rubygems/dependency_resolver/index_specification.rb trunk/lib/rubygems/dependency_resolver.rb trunk/lib/rubygems/remote_fetcher.rb trunk/lib/rubygems/source/local.rb trunk/lib/rubygems/specification.rb trunk/lib/rubygems/version.rb trunk/test/rubygems/test_gem_requirement.rb trunk/test/rubygems/test_gem_specification.rb trunk/test/rubygems/test_gem_version.rb Index: ChangeLog =================================================================== --- ChangeLog (revision 43038) +++ ChangeLog (revision 43039) @@ -1,3 +1,9 @@ https://github.com/ruby/ruby/blob/trunk/ChangeLog#L1 +Wed Sep 25 09:53:11 2013 Eric Hodel <drbrain@s...> + + * lib/rubygems: Fix CVE-2013-4363. Miscellaneous minor improvements. + + * test/rubygems: Tests for the above. + Tue Sep 24 17:38:56 2013 Nobuyoshi Nakada <nobu@r...> * string.c (rb_str_inspect): get rid of out-of-bound access. Index: lib/rubygems/dependency_resolver/index_specification.rb =================================================================== --- lib/rubygems/dependency_resolver/index_specification.rb (revision 43038) +++ lib/rubygems/dependency_resolver/index_specification.rb (revision 43039) @@ -1,8 +1,7 @@ https://github.com/ruby/ruby/blob/trunk/lib/rubygems/dependency_resolver/index_specification.rb#L1 ## -# Represents a possible Specification object returned -# from IndexSet. Used to delay needed to download full -# Specification objects when only the +name+ and +version+ -# are needed. +# Represents a possible Specification object returned from IndexSet. Used to +# delay needed to download full Specification objects when only the +name+ +# and +version+ are needed. class Gem::DependencyResolver::IndexSpecification Index: lib/rubygems/specification.rb =================================================================== --- lib/rubygems/specification.rb (revision 43038) +++ lib/rubygems/specification.rb (revision 43039) @@ -1509,7 +1509,6 @@ class Gem::Specification < Gem::BasicSpe https://github.com/ruby/ruby/blob/trunk/lib/rubygems/specification.rb#L1509 # [depending_gem, dependency, [list_of_gems_that_satisfy_dependency]] def dependent_gems - # REFACTOR: out = []; each; out; ? Really? No #collect love? out = [] Gem::Specification.each do |spec| spec.dependencies.each do |dep| @@ -1937,7 +1936,6 @@ class Gem::Specification < Gem::BasicSpe https://github.com/ruby/ruby/blob/trunk/lib/rubygems/specification.rb#L1936 q.group 2, 'Gem::Specification.new do |s|', 'end' do q.breakable - # REFACTOR: each_attr - use in to_yaml as well @@attributes.each do |attr_name| current_value = self.send attr_name if current_value != default_value(attr_name) or @@ -2186,10 +2184,6 @@ class Gem::Specification < Gem::BasicSpe https://github.com/ruby/ruby/blob/trunk/lib/rubygems/specification.rb#L2184 # Returns a Ruby code representation of this specification, such that it can # be eval'ed and reconstruct the same specification later. Attributes that # still have their default values are omitted. - # - # REFACTOR: This, plus stuff like #ruby_code and #pretty_print, should - # probably be extracted out into some sort of separate class. SRP, do you - # speak it!??! def to_ruby mark_version Index: lib/rubygems/source/local.rb =================================================================== --- lib/rubygems/source/local.rb (revision 43038) +++ lib/rubygems/source/local.rb (revision 43039) @@ -88,7 +88,7 @@ class Gem::Source::Local < Gem::Source https://github.com/ruby/ruby/blob/trunk/lib/rubygems/source/local.rb#L88 end end - found.sort_by { |s| s.version }.last + found.max_by { |s| s.version } end def fetch_spec(name) Index: lib/rubygems/commands/specification_command.rb =================================================================== --- lib/rubygems/commands/specification_command.rb (revision 43038) +++ lib/rubygems/commands/specification_command.rb (revision 43039) @@ -127,7 +127,7 @@ Specific fields in the specification can https://github.com/ruby/ruby/blob/trunk/lib/rubygems/commands/specification_command.rb#L127 end unless options[:all] then - specs = [specs.sort_by { |s| s.version }.last] + specs = [specs.max_by { |s| s.version }] end specs.each do |s| Index: lib/rubygems/commands/unpack_command.rb =================================================================== --- lib/rubygems/commands/unpack_command.rb (revision 43038) +++ lib/rubygems/commands/unpack_command.rb (revision 43039) @@ -134,7 +134,7 @@ command help for an example. https://github.com/ruby/ruby/blob/trunk/lib/rubygems/commands/unpack_command.rb#L134 specs = dependency.matching_specs - selected = specs.sort_by { |s| s.version }.last # HACK: hunt last down + selected = specs.max_by { |s| s.version } return Gem::RemoteFetcher.fetcher.download_to_cache(dependency) unless selected Index: lib/rubygems/commands/update_command.rb =================================================================== --- lib/rubygems/commands/update_command.rb (revision 43038) +++ lib/rubygems/commands/update_command.rb (revision 43039) @@ -134,7 +134,7 @@ command to remove old versions. https://github.com/ruby/ruby/blob/trunk/lib/rubygems/commands/update_command.rb#L134 g.name == spec.name and g.match_platform? end - highest_remote_gem = matching_gems.sort_by { |g,_| g.version }.last + highest_remote_gem = matching_gems.max_by { |g,_| g.version } highest_remote_gem ||= [Gem::NameTuple.null] Index: lib/rubygems/dependency_resolver.rb =================================================================== --- lib/rubygems/dependency_resolver.rb (revision 43038) +++ lib/rubygems/dependency_resolver.rb (revision 43039) @@ -92,6 +92,32 @@ class Gem::DependencyResolver https://github.com/ruby/ruby/blob/trunk/lib/rubygems/dependency_resolver.rb#L92 res.to_a end + ## + # Finds the State in +states+ that matches the +conflict+ so that we can try + # other possible sets. + + def find_conflict_state conflict, states # :nodoc: + until states.empty? do + if conflict.for_spec? states.last.spec + state = states.last + state.conflicts << [state.spec, conflict] + return state + else + states.pop + end + end + + nil + end + + ## + # Extracts the specifications that may be able to fulfill +dependency+ + + def find_possible dependency # :nodoc: + possible = @set.find_all dependency + select_local_platforms possible + end + def handle_conflict(dep, existing) # There is a conflict! We return the conflict # object which will be seen by the caller and be @@ -144,116 +170,119 @@ class Gem::DependencyResolver https://github.com/ruby/ruby/blob/trunk/lib/rubygems/dependency_resolver.rb#L170 # If there is already a spec activated for the requested name... if specs && existing = specs.find { |s| dep.name == s.name } - - # then we're done since this new dep matches the - # existing spec. + # then we're done since this new dep matches the existing spec. next if dep.matches_spec? existing conflict = handle_conflict dep, existing - # Look through the state array and pop State objects - # until we get back to the State that matches the conflict - # so that we can try other possible sets. - - i = nil - - until states.empty? - if conflict.for_spec? states.last.spec - i = states.last - i.conflicts << [i.spec, conflict] - break - else - states.pop - end - end - - if i - # We exhausted the possibles so it's definitely not going to - # work out, bail out. - - if i.possibles.empty? - raise Gem::ImpossibleDependenciesError.new(i.dep, i.conflicts) - end - - spec = i.possibles.pop - - # Recursively call #resolve_for with this spec - # and add it's dependencies into the picture... - - act = Gem::DependencyResolver::ActivationRequest.new spec, i.dep - - needed = requests(spec, act, i.needed) - specs = Gem::List.prepend(i.specs, act) - - next - else - return conflict - end + state = find_conflict_state conflict, states + + return conflict unless state + + needed, specs = resolve_for_conflict needed, specs, state + + next end - # Get a list of all specs that satisfy dep and platform - possible = @set.find_all dep - possible = select_local_platforms possible + possible = find_possible dep case possible.size when 0 - @missing << dep - - unless @soft_missing - # If there are none, then our work here is done. - raise Gem::UnsatisfiableDependencyError, dep - end + resolve_for_zero dep when 1 - # If there is one, then we just add it to specs - # and process the specs dependencies by adding - # them to needed. - - spec = possible.first - act = Gem::DependencyResolver::ActivationRequest.new spec, dep, false - - specs = Gem::List.prepend specs, act - - # Put the deps for at the beginning of needed - # rather than the end to match the depth first - # searching done by the multiple case code below. - # - # This keeps the error messages consistent. - needed = requests(spec, act, needed) + needed, specs = + resolve_for_single needed, specs, dep, possible else - # There are multiple specs for this dep. This is - # the case that this class is built to handle. + needed, specs = + resolve_for_multiple needed, specs, states, dep, possible + end + end + + specs + end - # Sort them so that we try the highest versions - # first. - possible = possible.sort_by do |s| - [s.source, s.version, s.platform == Gem::Platform::RUBY ? -1 : 1] - end - - # To figure out which to pick, we keep resolving - # given each one being activated and if there isn't - # a conflict, we know we've found a full set. - # - # We use an until loop rather than #reverse_each - # to keep the stack short since we're using a recursive - # algorithm. - # - spec = possible.pop - - # We're may need to try all of +possible+, so we setup - # state to unwind back to current +needed+ and +specs+ - # so we can try another. This is code is what makes the above - # code in conflict resolution possible. + ## + # Rewinds +needed+ and +specs+ to a previous state in +state+ for a conflict + # between +dep+ and +existing+. - act = Gem::DependencyResolver::ActivationRequest.new spec, dep + def resolve_for_conflict needed, specs, state # :nodoc: + # We exhausted the possibles so it's definitely not going to work out, + # bail out. + raise Gem::ImpossibleDependenciesError.new state.dep, state.conflicts if + state.possibles.empty? - states << State.new(needed, specs, dep, spec, possible, []) + spec = state.possibles.pop - needed = requests(spec, act, needed) - specs = Gem::List.prepend(specs, act) - end + # Retry resolution with this spec and add it's dependencies + act = Gem::DependencyResolver::ActivationRequest.new spec, state.dep + + needed = requests spec, act, state.needed + specs = Gem::List.prepend state.specs, act + + return needed, specs + end + + ## + # There are multiple +possible+ specifications for this +dep+. Updates + # +needed+, +specs+ and +states+ for further resolution of the +possible+ + # choices. + + def resolve_for_multiple needed, specs, states, dep, possible # :nodoc: + # Sort them so that we try the highest versions first. + possible = possible.sort_by do |s| + [s.source, s.version, s.platform == Gem::Platform::RUBY ? -1 : 1] end - specs + # To figure out which to pick, we keep resolving given each one being + # activated and if there isn't a conflict, we know we've found a full set. + # + # We use an until loop rather than reverse_each to keep the stack short + # since we're using a recursive algorithm. + spec = possible.pop + + # We may need to try all of +possible+, so we setup state to unwind back + # to current +needed+ and +specs+ so we can try another. This is code is + # what makes conflict resolution possible. + + act = Gem::DependencyResolver::ActivationRequest.new spec, dep + + states << State.new(needed, specs, dep, spec, possible, []) + + needed = requests spec, act, needed + specs = Gem::List.prepend specs, act + + return needed, specs + end + + ## + # Add the spec from the +possible+ list to +specs+ and process the spec's + # dependencies by adding them to +needed+. + + def resolve_for_single needed, specs, dep, possible # :nodoc: + spec = possible.first + act = Gem::DependencyResolver::ActivationRequest.new spec, dep, false + + specs = Gem::List.prepend specs, act + + # Put the deps for at the beginning of needed + # rather than the end to match the depth first + # searching done by the multiple case code below. + # + # This keeps the error messages consistent. + needed = requests spec, act, needed + + return needed, specs + end + + ## + # When there are no possible specifications for +dep+ our work is done. + + def resolve_for_zero dep # :nodoc: + @missing << dep + + unless @soft_missing + raise Gem::UnsatisfiableDependencyError, dep + end end ## Index: lib/rubygems/version.rb =================================================================== --- lib/rubygems/version.rb (revision 43038) +++ lib/rubygems/version.rb (revision 43039) @@ -148,7 +148,7 @@ class Gem::Version https://github.com/ruby/ruby/blob/trunk/lib/rubygems/version.rb#L148 # FIX: These are only used once, in .correct?. Do they deserve to be # constants? VERSION_PATTERN = '[0-9]+(?>\.[0-9a-zA-Z]+)*(-[0-9A-Za-z-]+(\.[0-9A-Za-z-]+)*)?' # :nodoc: - ANCHORED_VERSION_PATTERN = /\A\s*(#{VERSION_PATTERN})*\s*\z/ # :nodoc: + ANCHORED_VERSION_PATTERN = /\A\s*(#{VERSION_PATTERN})?\s*\z/ # :nodoc: ## # A string representation of this Version. Index: lib/rubygems/remote_fetcher.rb =================================================================== --- lib/rubygems/remote_fetcher.rb (revision 43038) +++ lib/rubygems/remote_fetcher.rb (revision 43039) @@ -107,7 +107,7 @@ class Gem::RemoteFetcher https://github.com/ruby/ruby/blob/trunk/lib/rubygems/remote_fetcher.rb#L107 return if found.empty? - spec, source = found.sort_by { |(s,_)| s.version }.last + spec, source = found.max_by { |(s,_)| s.version } download spec, source.uri.to_s end Index: test/rubygems/test_gem_specification.rb =================================================================== --- test/rubygems/test_gem_specification.rb (revision 43038) +++ test/rubygems/test_gem_specification.rb (revision 43039) @@ -1112,6 +1112,20 @@ dependencies: [] https://github.com/ruby/ruby/blob/trunk/test/rubygems/test_gem_specification.rb#L1112 assert_equal [@bonobo, @monkey], @gem.dependencies end + def test_dependent_gems + util_setup_deps + + assert_empty @gem.dependent_gems + + bonobo = quick_spec 'bonobo' + + expected = [ + [@gem, @bonobo, [bonobo]], + ] + + assert_equal expected, bonobo.dependent_gems + end + def test_doc_dir assert_equal File.join(@gemhome, 'doc', 'a-1'), @a1.doc_dir end @@ -1550,13 +1564,13 @@ dependencies: [] https://github.com/ruby/ruby/blob/trunk/test/rubygems/test_gem_specification.rb#L1564 @a2.add_runtime_dependency 'b', '1' @a2.dependencies.first.instance_variable_set :@type, nil @a2.required_rubygems_version = Gem::Requirement.new '> 0' - @a2.require_paths << "lib/a/ext" + @a2.require_paths << 'other' ruby_code = @a2.to_ruby expected = <<-SPEC # -*- encoding: utf-8 -*- -# stub: a 2 ruby lib\0lib/a/ext +# stub: a 2 ruby lib\0other Gem::Specification.new do |s| s.name = "a" @@ -1569,7 +1583,7 @@ Gem::Specification.new do |s| https://github.com/ruby/ruby/blob/trunk/test/rubygems/test_gem_specification.rb#L1583 s.email = "example@e..." s.files = ["lib/code.rb"] s.homepage = "http://example.com" - s.require_paths = ["lib", "lib/a/ext"] + s.require_paths = ["lib", "other"] s.rubygems_version = "#{Gem::VERSION}" s.summary = "this is a summary" Index: test/rubygems/test_gem_version.rb =================================================================== --- test/rubygems/test_gem_version.rb (revision 43038) +++ test/rubygems/test_gem_version.rb (revision 43039) @@ -67,12 +67,17 @@ class TestGemVersion < Gem::TestCase https://github.com/ruby/ruby/blob/trunk/test/rubygems/test_gem_version.rb#L67 end def test_initialize_bad - ["junk", "1.0\n2.0"].each do |bad| - e = assert_raises ArgumentError do + %W[ + junk + 1.0\n2.0 + 1..2 + 1.2\ 3.4 + ].each do |bad| + e = assert_raises ArgumentError, bad do Gem::Version.new bad end - assert_equal "Malformed version number string #{bad}", e.message + assert_equal "Malformed version number string #{bad}", e.message, bad end end Index: test/rubygems/test_gem_requirement.rb =================================================================== --- test/rubygems/test_gem_requirement.rb (revision 43038) +++ test/rubygems/test_gem_requirement.rb (revision 43039) @@ -47,18 +47,20 @@ class TestGemRequirement < Gem::TestCase https://github.com/ruby/ruby/blob/trunk/test/rubygems/test_gem_requirement.rb#L47 end def test_parse_bad - e = assert_raises Gem::Requirement::BadRequirementError do - Gem::Requirement.parse nil - end - - assert_equal 'Illformed requirement [nil]', e.message + [ + nil, + '', + '! 1', + '= junk', + '1..2', + ].each do |bad| + e = assert_raises Gem::Requirement::BadRequirementError do + Gem::Requirement.parse bad + end - e = assert_raises Gem::Requirement::BadRequirementError do - Gem::Requirement.parse "" + assert_equal "Illformed requirement [#{bad.inspect}]", e.message end - assert_equal 'Illformed requirement [""]', e.message - assert_equal Gem::Requirement::BadRequirementError.superclass, ArgumentError end -- ML: ruby-changes@q... Info: http://www.atdot.net/~ko1/quickml/