summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorusa <usa@b2dd03c8-39d4-4d8f-98ff-823fe69b080e>2019-03-18 18:30:36 +0000
committerusa <usa@b2dd03c8-39d4-4d8f-98ff-823fe69b080e>2019-03-18 18:30:36 +0000
commitf86e5daee790ee509cb17f4f51f95cc76ca89a4e (patch)
treebd949f04a3e8551bc3ce032e328b6e5ca724ec8f
parent391f88c8431075596a6f44d8bea19d2e155e3003 (diff)
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
-rw-r--r--lib/rubygems.rb2
-rw-r--r--lib/rubygems/command_manager.rb10
-rw-r--r--lib/rubygems/commands/owner_command.rb5
-rw-r--r--lib/rubygems/gemcutter_utilities.rb7
-rw-r--r--lib/rubygems/installer.rb29
-rw-r--r--lib/rubygems/package.rb10
-rw-r--r--lib/rubygems/user_interaction.rb7
-rw-r--r--test/rubygems/test_gem_installer.rb108
-rw-r--r--test/rubygems/test_gem_package.rb36
-rw-r--r--test/rubygems/test_gem_text.rb5
-rw-r--r--version.h6
11 files changed, 207 insertions, 18 deletions
diff --git a/lib/rubygems.rb b/lib/rubygems.rb
index 4ab0da9acf..f1756cb45d 100644
--- a/lib/rubygems.rb
+++ b/lib/rubygems.rb
@@ -10,7 +10,7 @@ require 'rbconfig'
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 451b719c46..d3ff6614dc 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 @@ require 'rubygems/user_interaction'
class Gem::CommandManager
+ include Gem::Text
include Gem::UserInteraction
BUILTIN_COMMANDS = [ # :nodoc:
@@ -138,12 +140,12 @@ class Gem::CommandManager
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 @@ class Gem::CommandManager
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 @@ class Gem::CommandManager
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 2ee7f84462..7842a322cf 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 @@ permission to.
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 7c6d6bb364..623d9301b5 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 @@ module Gem::GemcutterUtilities
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 6fd3399dd4..5818b94fb5 100644
--- a/lib/rubygems/installer.rb
+++ b/lib/rubygems/installer.rb
@@ -697,9 +697,26 @@ class Gem::Installer
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 @@ TEXT
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 1dd28d8132..d2cbd09134 100644
--- a/lib/rubygems/package.rb
+++ b/lib/rubygems/package.rb
@@ -425,6 +425,16 @@ EOM
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 390d0f2aea..237ae2bc71 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 @@ end
module Gem::DefaultUserInteraction
+ include Gem::Text
+
##
# The default UI is a class variable of the singleton class for this
# module.
@@ -165,8 +168,8 @@ module Gem::UserInteraction
# 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 dd049214fb..af4573cde8 100644
--- a/test/rubygems/test_gem_installer.rb
+++ b/test/rubygems/test_gem_installer.rb
@@ -1468,6 +1468,114 @@ gem 'other', version
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 "#<Gem::Specification name=malicious\n::Object.const_set(:FROM_EVAL, true)# version=1> 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 "#<Gem::Specification name=malicious version=1> 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 "#<Gem::Specification name=malicious version=1> 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 "#<Gem::Specification name=malicious version=1> 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 "#<Gem::Specification name=malicious version=1> 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 713c84d700..1fbc32b298 100644
--- a/test/rubygems/test_gem_package.rb
+++ b/test/rubygems/test_gem_package.rb
@@ -480,6 +480,42 @@ class TestGemPackage < Gem::Package::TarTestCase
"#{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 04f3f605e8..8ce6df94bb 100644
--- a/test/rubygems/test_gem_text.rb
+++ b/test/rubygems/test_gem_text.rb
@@ -85,4 +85,9 @@ Without the wrapping, the text might not look good in the RSS feed.
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 310cd21271..32f0e9abfa 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"