From f86e5daee790ee509cb17f4f51f95cc76ca89a4e Mon Sep 17 00:00:00 2001 From: usa Date: Mon, 18 Mar 2019 18:30:36 +0000 Subject: [PATCH] Applied security patches for RubyGems git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/branches/ruby_2_4@67303 b2dd03c8-39d4-4d8f-98ff-823fe69b080e --- lib/rubygems.rb | 2 +- lib/rubygems/command_manager.rb | 10 ++- lib/rubygems/commands/owner_command.rb | 5 +- lib/rubygems/gemcutter_utilities.rb | 7 +- lib/rubygems/installer.rb | 29 +++++-- lib/rubygems/package.rb | 10 +++ lib/rubygems/user_interaction.rb | 7 +- test/rubygems/test_gem_installer.rb | 108 +++++++++++++++++++++++++ test/rubygems/test_gem_package.rb | 36 +++++++++ test/rubygems/test_gem_text.rb | 5 ++ version.h | 6 +- 11 files changed, 207 insertions(+), 18 deletions(-) diff --git a/lib/rubygems.rb b/lib/rubygems.rb index 4ab0da9acfe086..f1756cb45d938b 100644 --- a/lib/rubygems.rb +++ b/lib/rubygems.rb @@ -10,7 +10,7 @@ require 'thread' module Gem - VERSION = "2.6.14.3" + VERSION = "2.6.14.4" end # Must be first since it unloads the prelude from 1.9.2 diff --git a/lib/rubygems/command_manager.rb b/lib/rubygems/command_manager.rb index 451b719c4683e1..d3ff6614dc472f 100644 --- a/lib/rubygems/command_manager.rb +++ b/lib/rubygems/command_manager.rb @@ -7,6 +7,7 @@ require 'rubygems/command' require 'rubygems/user_interaction' +require 'rubygems/text' ## # The command manager registers and installs all the individual sub-commands @@ -32,6 +33,7 @@ class Gem::CommandManager + include Gem::Text include Gem::UserInteraction BUILTIN_COMMANDS = [ # :nodoc: @@ -138,12 +140,12 @@ def command_names def run(args, build_args=nil) process_args(args, build_args) rescue StandardError, Timeout::Error => ex - alert_error "While executing gem ... (#{ex.class})\n #{ex}" + alert_error clean_text("While executing gem ... (#{ex.class})\n #{ex}") ui.backtrace ex terminate_interaction(1) rescue Interrupt - alert_error "Interrupted" + alert_error clean_text("Interrupted") terminate_interaction(1) end @@ -161,7 +163,7 @@ def process_args(args, build_args=nil) say Gem::VERSION terminate_interaction 0 when /^-/ then - alert_error "Invalid option: #{args.first}. See 'gem --help'." + alert_error clean_text("Invalid option: #{args.first}. See 'gem --help'.") terminate_interaction 1 else cmd_name = args.shift.downcase @@ -210,7 +212,7 @@ def load_and_instantiate(command_name) rescue Exception => e e = load_error if load_error - alert_error "Loading command: #{command_name} (#{e.class})\n\t#{e}" + alert_error clean_text("Loading command: #{command_name} (#{e.class})\n\t#{e}") ui.backtrace e end end diff --git a/lib/rubygems/commands/owner_command.rb b/lib/rubygems/commands/owner_command.rb index 2ee7f84462c12d..7842a322cfce2b 100644 --- a/lib/rubygems/commands/owner_command.rb +++ b/lib/rubygems/commands/owner_command.rb @@ -2,8 +2,11 @@ require 'rubygems/command' require 'rubygems/local_remote_options' require 'rubygems/gemcutter_utilities' +require 'rubygems/text' class Gem::Commands::OwnerCommand < Gem::Command + + include Gem::Text include Gem::LocalRemoteOptions include Gem::GemcutterUtilities @@ -62,7 +65,7 @@ def show_owners name end with_response response do |resp| - owners = Gem::SafeYAML.load resp.body + owners = Gem::SafeYAML.load clean_text(resp.body) say "Owners for gem: #{name}" owners.each do |owner| diff --git a/lib/rubygems/gemcutter_utilities.rb b/lib/rubygems/gemcutter_utilities.rb index 7c6d6bb36404ae..623d9301b5982f 100644 --- a/lib/rubygems/gemcutter_utilities.rb +++ b/lib/rubygems/gemcutter_utilities.rb @@ -1,11 +1,14 @@ # frozen_string_literal: true require 'rubygems/remote_fetcher' +require 'rubygems/text' ## # Utility methods for using the RubyGems API. module Gem::GemcutterUtilities + include Gem::Text + # TODO: move to Gem::Command OptionParser.accept Symbol do |value| value.to_sym @@ -145,13 +148,13 @@ def with_response response, error_prefix = nil if block_given? then yield response else - say response.body + say clean_text(response.body) end else message = response.body message = "#{error_prefix}: #{message}" if error_prefix - say message + say clean_text(message) terminate_interaction 1 # TODO: question this end end diff --git a/lib/rubygems/installer.rb b/lib/rubygems/installer.rb index 6fd3399dd44c30..5818b94fb5f86f 100644 --- a/lib/rubygems/installer.rb +++ b/lib/rubygems/installer.rb @@ -697,9 +697,26 @@ def verify_gem_home(unpack = false) # :nodoc: unpack or File.writable?(gem_home) end - def verify_spec_name - return if spec.name =~ Gem::Specification::VALID_NAME_PATTERN - raise Gem::InstallError, "#{spec} has an invalid name" + def verify_spec + unless spec.name =~ Gem::Specification::VALID_NAME_PATTERN + raise Gem::InstallError, "#{spec} has an invalid name" + end + + if spec.raw_require_paths.any?{|path| path =~ /\r\n|\r|\n/ } + raise Gem::InstallError, "#{spec} has an invalid require_paths" + end + + if spec.extensions.any?{|ext| ext =~ /\r\n|\r|\n/ } + raise Gem::InstallError, "#{spec} has an invalid extensions" + end + + unless spec.specification_version.to_s =~ /\A\d+\z/ + raise Gem::InstallError, "#{spec} has an invalid specification_version" + end + + if spec.dependencies.any? {|dep| dep.type =~ /\r\n|\r|\n/ || dep.name =~ /\r\n|\r|\n/ } + raise Gem::InstallError, "#{spec} has an invalid dependencies" + end end ## @@ -826,9 +843,11 @@ def dir def pre_install_checks verify_gem_home options[:unpack] - ensure_loadable_spec + # The name and require_paths must be verified first, since it could contain + # ruby code that would be eval'ed in #ensure_loadable_spec + verify_spec - verify_spec_name + ensure_loadable_spec if options[:install_as_default] Gem.ensure_default_gem_subdirectories gem_home diff --git a/lib/rubygems/package.rb b/lib/rubygems/package.rb index 1dd28d81322d4c..d2cbd0913443a7 100644 --- a/lib/rubygems/package.rb +++ b/lib/rubygems/package.rb @@ -425,6 +425,16 @@ def install_location filename, destination_dir # :nodoc: raise Gem::Package::PathError.new(destination, destination_dir) unless destination.start_with? destination_dir + '/' + begin + real_destination = File.expand_path(File.realpath(destination)) + rescue + # it's fine if the destination doesn't exist, because rm -rf'ing it can't cause any damage + nil + else + raise Gem::Package::PathError.new(real_destination, destination_dir) unless + real_destination.start_with? destination_dir + '/' + end + destination.untaint destination end diff --git a/lib/rubygems/user_interaction.rb b/lib/rubygems/user_interaction.rb index 390d0f2aea72b5..237ae2bc71c203 100644 --- a/lib/rubygems/user_interaction.rb +++ b/lib/rubygems/user_interaction.rb @@ -6,6 +6,7 @@ #++ require 'rubygems/util' +require 'rubygems/text' begin require 'io/console' @@ -18,6 +19,8 @@ module Gem::DefaultUserInteraction + include Gem::Text + ## # The default UI is a class variable of the singleton class for this # module. @@ -165,8 +168,8 @@ def terminate_interaction exit_code = 0 # Calls +say+ with +msg+ or the results of the block if really_verbose # is true. - def verbose msg = nil - say(msg || yield) if Gem.configuration.really_verbose + def verbose(msg = nil) + say(clean_text(msg || yield)) if Gem.configuration.really_verbose end end diff --git a/test/rubygems/test_gem_installer.rb b/test/rubygems/test_gem_installer.rb index dd049214fbb8af..af4573cde8d2d8 100644 --- a/test/rubygems/test_gem_installer.rb +++ b/test/rubygems/test_gem_installer.rb @@ -1468,6 +1468,114 @@ def spec.validate; end end end + def test_pre_install_checks_malicious_name_before_eval + spec = util_spec "malicious\n::Object.const_set(:FROM_EVAL, true)#", '1' + def spec.full_name # so the spec is buildable + "malicious-1" + end + def spec.validate(*args); end + + util_build_gem spec + + gem = File.join(@gemhome, 'cache', spec.file_name) + + use_ui @ui do + @installer = Gem::Installer.at gem + e = assert_raises Gem::InstallError do + @installer.pre_install_checks + end + assert_equal "# has an invalid name", e.message + end + refute defined?(::Object::FROM_EVAL) + end + + def test_pre_install_checks_malicious_require_paths_before_eval + spec = util_spec "malicious", '1' + def spec.full_name # so the spec is buildable + "malicious-1" + end + def spec.validate(*args); end + spec.require_paths = ["malicious\n``"] + + util_build_gem spec + + gem = File.join(@gemhome, 'cache', spec.file_name) + + use_ui @ui do + @installer = Gem::Installer.at gem + e = assert_raises Gem::InstallError do + @installer.pre_install_checks + end + assert_equal "# has an invalid require_paths", e.message + end + end + + def test_pre_install_checks_malicious_extensions_before_eval + skip "mswin environment disallow to create file contained the carriage return code." if Gem.win_platform? + + spec = util_spec "malicious", '1' + def spec.full_name # so the spec is buildable + "malicious-1" + end + def spec.validate(*args); end + spec.extensions = ["malicious\n``"] + + util_build_gem spec + + gem = File.join(@gemhome, 'cache', spec.file_name) + + use_ui @ui do + @installer = Gem::Installer.at gem + e = assert_raises Gem::InstallError do + @installer.pre_install_checks + end + assert_equal "# has an invalid extensions", e.message + end + end + + def test_pre_install_checks_malicious_specification_version_before_eval + spec = util_spec "malicious", '1' + def spec.full_name # so the spec is buildable + "malicious-1" + end + def spec.validate(*args); end + spec.specification_version = "malicious\n``" + + util_build_gem spec + + gem = File.join(@gemhome, 'cache', spec.file_name) + + use_ui @ui do + @installer = Gem::Installer.at gem + e = assert_raises Gem::InstallError do + @installer.pre_install_checks + end + assert_equal "# has an invalid specification_version", e.message + end + end + + def test_pre_install_checks_malicious_dependencies_before_eval + spec = util_spec "malicious", '1' + def spec.full_name # so the spec is buildable + "malicious-1" + end + def spec.validate(*args); end + spec.add_dependency "b\nfoo", '> 5' + + util_build_gem spec + + gem = File.join(@gemhome, 'cache', spec.file_name) + + use_ui @ui do + @installer = Gem::Installer.at gem + @installer.ignore_dependencies = true + e = assert_raises Gem::InstallError do + @installer.pre_install_checks + end + assert_equal "# has an invalid dependencies", e.message + end + end + def test_shebang util_make_exec @spec, "#!/usr/bin/ruby" diff --git a/test/rubygems/test_gem_package.rb b/test/rubygems/test_gem_package.rb index 713c84d700b080..1fbc32b2987465 100644 --- a/test/rubygems/test_gem_package.rb +++ b/test/rubygems/test_gem_package.rb @@ -480,6 +480,42 @@ def test_extract_symlink_parent "#{destination_subdir} is not allowed", e.message) end + def test_extract_symlink_parent_doesnt_delete_user_dir + skip if RUBY_VERSION <= "1.8.7" + + package = Gem::Package.new @gem + + # 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 + + destination_user_dir = File.join @destination, 'user' + destination_user_subdir = File.join destination_user_dir, 'dir' + FileUtils.mkdir_p destination_user_subdir + + tgz_io = util_tar_gz do |tar| + tar.add_symlink 'link', destination_user_dir, 16877 + tar.add_symlink 'link/dir', '.', 16877 + end + + e = assert_raises(Gem::Package::PathError, Errno::EACCES) do + package.extract_tar_gz tgz_io, destination_subdir + end + + assert_path_exists destination_user_subdir + + if Gem::Package::PathError === e + assert_equal("installing into parent path #{destination_user_subdir} of " + + "#{destination_subdir} is not allowed", e.message) + elsif win_platform? + skip "symlink - must be admin with no UAC on Windows" + else + raise e + end + end + def test_extract_tar_gz_directory package = Gem::Package.new @gem diff --git a/test/rubygems/test_gem_text.rb b/test/rubygems/test_gem_text.rb index 04f3f605e8c037..8ce6df94bbc04a 100644 --- a/test/rubygems/test_gem_text.rb +++ b/test/rubygems/test_gem_text.rb @@ -85,4 +85,9 @@ def test_truncate_text s = "ab" * 500_001 assert_equal "Truncating desc to 1,000,000 characters:\n#{s[0, 1_000_000]}", truncate_text(s, "desc", 1_000_000) end + + def test_clean_text + assert_equal ".]2;nyan.", clean_text("\e]2;nyan\a") + end + end diff --git a/version.h b/version.h index 310cd21271d49a..32f0e9abfa6a1b 100644 --- a/version.h +++ b/version.h @@ -1,10 +1,10 @@ #define RUBY_VERSION "2.4.6" -#define RUBY_RELEASE_DATE "2019-03-01" -#define RUBY_PATCHLEVEL 351 +#define RUBY_RELEASE_DATE "2019-03-19" +#define RUBY_PATCHLEVEL 352 #define RUBY_RELEASE_YEAR 2019 #define RUBY_RELEASE_MONTH 3 -#define RUBY_RELEASE_DAY 1 +#define RUBY_RELEASE_DAY 19 #include "ruby/version.h"