diff --git a/lib/rubygems/package.rb b/lib/rubygems/package.rb index 7f843f4e0a25..0ca272e09974 100644 --- a/lib/rubygems/package.rb +++ b/lib/rubygems/package.rb @@ -71,6 +71,13 @@ def initialize(destination, destination_dir) end end + class SymlinkError < Error + def initialize(name, destination, destination_dir) + super "installing symlink '%s' pointing to parent path %s of %s is not allowed" % + [name, destination, destination_dir] + end + end + class NonSeekableIO < Error; end class TooLongFileName < Error; end @@ -407,6 +414,14 @@ def extract_tar_gz(io, destination_dir, pattern = "*") # :nodoc: destination = install_location entry.full_name, destination_dir + if entry.symlink? + link_target = entry.header.linkname + real_destination = link_target.start_with?("/") ? link_target : File.expand_path(link_target, File.dirname(destination)) + + raise Gem::Package::SymlinkError.new(entry.full_name, real_destination, destination_dir) unless + normalize_path(real_destination).start_with? normalize_path(destination_dir + '/') + end + FileUtils.rm_rf destination mkdir_options = {} @@ -419,7 +434,7 @@ def extract_tar_gz(io, destination_dir, pattern = "*") # :nodoc: end unless directories.include?(mkdir) - mkdir_p_safe mkdir, mkdir_options, destination_dir, entry.full_name + FileUtils.mkdir_p mkdir, **mkdir_options directories << mkdir end @@ -495,22 +510,6 @@ def normalize_path(pathname) end end - def mkdir_p_safe(mkdir, mkdir_options, destination_dir, file_name) - destination_dir = File.realpath(File.expand_path(destination_dir)) - parts = mkdir.split(File::SEPARATOR) - parts.reduce do |path, basename| - path = File.realpath(path) unless path == "" - path = File.expand_path(path + File::SEPARATOR + basename) - lstat = File.lstat path rescue nil - if !lstat || !lstat.directory? - unless normalize_path(path).start_with? normalize_path(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+ diff --git a/test/rubygems/test_gem_package.rb b/test/rubygems/test_gem_package.rb index 5dabcfd47c1e..27afca1ccbf8 100644 --- a/test/rubygems/test_gem_package.rb +++ b/test/rubygems/test_gem_package.rb @@ -574,7 +574,7 @@ def test_extract_symlink_parent destination_subdir = File.join @destination, 'subdir' FileUtils.mkdir_p destination_subdir - expected_exceptions = win_platform? ? [Gem::Package::PathError, Errno::EACCES] : [Gem::Package::PathError] + expected_exceptions = win_platform? ? [Gem::Package::SymlinkError, Errno::EACCES] : [Gem::Package::SymlinkError] e = assert_raise(*expected_exceptions) do package.extract_tar_gz tgz_io, destination_subdir @@ -582,10 +582,11 @@ def test_extract_symlink_parent pend "symlink - must be admin with no UAC on Windows" if Errno::EACCES === e - assert_equal("installing into parent path lib/link/outside.txt of " + + assert_equal("installing symlink 'lib/link' pointing to parent path #{@destination} of " + "#{destination_subdir} is not allowed", e.message) assert_path_not_exist File.join(@destination, "outside.txt") + assert_path_not_exist File.join(destination_subdir, "lib/link") end def test_extract_symlink_parent_doesnt_delete_user_dir @@ -608,7 +609,7 @@ def test_extract_symlink_parent_doesnt_delete_user_dir tar.add_symlink 'link/dir', '.', 16877 end - expected_exceptions = win_platform? ? [Gem::Package::PathError, Errno::EACCES] : [Gem::Package::PathError] + expected_exceptions = win_platform? ? [Gem::Package::SymlinkError, Errno::EACCES] : [Gem::Package::SymlinkError] e = assert_raise(*expected_exceptions) do package.extract_tar_gz tgz_io, destination_subdir @@ -616,10 +617,12 @@ def test_extract_symlink_parent_doesnt_delete_user_dir pend "symlink - must be admin with no UAC on Windows" if Errno::EACCES === e - assert_equal("installing into parent path #{destination_user_subdir} of " + + assert_equal("installing symlink 'link' pointing to parent path #{destination_user_dir} of " + "#{destination_subdir} is not allowed", e.message) assert_path_exist destination_user_subdir + assert_path_not_exist File.join(destination_subdir, "link/dir") + assert_path_not_exist File.join(destination_subdir, "link") end def test_extract_tar_gz_directory