Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 15 additions & 4 deletions lib/rubygems/package.rb
Original file line number Diff line number Diff line change
Expand Up @@ -453,10 +453,7 @@ def extract_tar_gz(io, destination_dir, pattern = "*") # :nodoc:
directories << mkdir
end

real_mkdir = File.realpath(mkdir)
unless real_mkdir == destination_dir || normalize_path(real_mkdir).start_with?(normalize_path(destination_dir + "/"))
raise Gem::Package::PathError.new(real_mkdir, destination_dir)
end
verify_extraction_dir mkdir, destination_dir

if entry.file?
File.open(destination, "wb") do |out|
Expand Down Expand Up @@ -662,6 +659,20 @@ def install_location(filename, destination_dir) # :nodoc:
destination
end

##
# Raises an exception unless +dir+, with symlinks resolved, is
# +destination_dir+ or a directory inside it. +destination_dir+ must
# already be resolved with File.realpath by the caller.

def verify_extraction_dir(dir, destination_dir) # :nodoc:
real_dir = File.realpath(dir)

return if real_dir == destination_dir ||
normalize_path(real_dir).start_with?(normalize_path(destination_dir + "/"))

raise Gem::Package::PathError.new(real_dir, destination_dir)
end

##
# Verifies the +checksums+ against the +digests+. This check is not
# cryptographically secure. Missing checksums are ignored.
Expand Down
6 changes: 4 additions & 2 deletions lib/rubygems/package/old.rb
Original file line number Diff line number Diff line change
Expand Up @@ -79,10 +79,12 @@ def extract_files(destination_dir)
raise Gem::Package::FormatError, "#{full_name} in #{@gem} is corrupt" if
file_data.length != entry["size"].to_i

FileUtils.rm_rf destination

FileUtils.mkdir_p File.dirname(destination), mode: dir_mode && 0o755

verify_extraction_dir File.dirname(destination), destination_dir

FileUtils.rm_rf destination

File.open destination, "wb", file_mode(entry["mode"]) do |out|
out.write file_data
end
Expand Down
16 changes: 16 additions & 0 deletions test/rubygems/test_gem_package_old.rb
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,22 @@ def test_extract_files
assert_equal mask, File.stat(extracted).mode unless Gem.win_platform?
end

def test_extract_files_rejects_preexisting_symlink_escape
omit "Symlinks not supported or not enabled" unless symlink_supported?

escape_dir = File.join @tempdir, "escape"
FileUtils.mkdir_p escape_dir

File.symlink escape_dir, File.join(@destination, "lib")

assert_raise Gem::Package::PathError do
@package.extract_files @destination
end

assert_path_not_exist File.join(escape_dir, "foo.rb"),
"must not write outside extraction root via symlink"
end

def test_extract_files_security_policy
pend "openssl is missing" unless Gem::HAVE_OPENSSL

Expand Down