1
0
Fork 0
mirror of https://github.com/ruby/ruby.git synced 2022-11-09 12:17:21 -05:00

[ruby/rubygems] Check safety of packaged symlinks

If we explicitly disallow the creation of symlinks that point to files
outside of the destination directory, we can avoid any other safety
checks while creating directories, because we can be sure they will
always fall under the destination directory as well.

https://github.com/rubygems/rubygems/commit/555692b8de
This commit is contained in:
David Rodríguez 2021-10-06 18:17:37 +02:00 committed by git
parent a5289bfa71
commit bbcf8f87ac
2 changed files with 23 additions and 21 deletions

View file

@ -71,6 +71,13 @@ class Gem::Package
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 @@ EOM
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 @@ EOM
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 @@ EOM
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+

View file

@ -574,7 +574,7 @@ class TestGemPackage < Gem::Package::TarTestCase
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 @@ class TestGemPackage < Gem::Package::TarTestCase
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 @@ class TestGemPackage < Gem::Package::TarTestCase
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 @@ class TestGemPackage < Gem::Package::TarTestCase
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