diff options
author | Kamil TrzciĆski <ayufan@ayufan.eu> | 2019-01-02 20:01:11 +0100 |
---|---|---|
committer | Yorick Peterse <yorickpeterse@gmail.com> | 2019-01-31 16:52:48 +0100 |
commit | 66744469d4f2c444c0248b84096d252db749d01c (patch) | |
tree | 0b71d2c71a195d61dca9b814e7fff31abe59004e /lib/safe_zip | |
parent | a1bf088201702ec4d36015c8f4cb635fa2ee2c5b (diff) | |
download | gitlab-ce-66744469d4f2c444c0248b84096d252db749d01c.tar.gz |
Extract GitLab Pages using RubyZip
RubyZip allows us to perform strong validation of
expanded paths where we do extract file.
We introduce the following additional checks
to extract routines:
1. None of path components can be symlinked,
2. We drop privileges support for directories,
3. Symlink source needs to point within the target directory,
like `public/`,
4. The symlink source needs to exist ahead of time.
Diffstat (limited to 'lib/safe_zip')
-rw-r--r-- | lib/safe_zip/entry.rb | 97 | ||||
-rw-r--r-- | lib/safe_zip/extract.rb | 73 | ||||
-rw-r--r-- | lib/safe_zip/extract_params.rb | 36 |
3 files changed, 206 insertions, 0 deletions
diff --git a/lib/safe_zip/entry.rb b/lib/safe_zip/entry.rb new file mode 100644 index 00000000000..664e2f52f91 --- /dev/null +++ b/lib/safe_zip/entry.rb @@ -0,0 +1,97 @@ +# frozen_string_literal: true + +module SafeZip + class Entry + attr_reader :zip_archive, :zip_entry + attr_reader :path, :params + + def initialize(zip_archive, zip_entry, params) + @zip_archive = zip_archive + @zip_entry = zip_entry + @params = params + @path = ::File.expand_path(zip_entry.name, params.extract_path) + end + + def path_dir + ::File.dirname(path) + end + + def real_path_dir + ::File.realpath(path_dir) + end + + def exist? + ::File.exist?(path) + end + + def extract + # do not extract if file is not part of target directory + return false unless matching_target_directory + + # do not overwrite existing file + raise SafeZip::Extract::AlreadyExistsError, "File already exists #{zip_entry.name}" if exist? + + create_path_dir + + if zip_entry.file? + extract_file + elsif zip_entry.directory? + extract_dir + elsif zip_entry.symlink? + extract_symlink + else + raise SafeZip::Extract::UnsupportedEntryError, "File #{zip_entry.name} cannot be extracted" + end + rescue SafeZip::Extract::Error + raise + rescue => e + raise SafeZip::Extract::ExtractError, e.message + end + + private + + def extract_file + zip_archive.extract(zip_entry, path) + end + + def extract_dir + FileUtils.mkdir(path) + end + + def extract_symlink + source_path = read_symlink + real_source_path = expand_symlink(source_path) + + # ensure that source path of symlink is within target directories + unless real_source_path.start_with?(matching_target_directory) + raise SafeZip::Extract::PermissionDeniedError, "Symlink cannot be created targeting: #{source_path}" + end + + ::File.symlink(source_path, path) + end + + def create_path_dir + # Create all directories, but ignore permissions + FileUtils.mkdir_p(path_dir) + + # disallow to make path dirs to point to another directories + unless path_dir == real_path_dir + raise SafeZip::Extract::PermissionDeniedError, "Directory of #{zip_entry.name} points to another directory" + end + end + + def matching_target_directory + params.matching_target_directory(path) + end + + def read_symlink + zip_archive.read(zip_entry) + end + + def expand_symlink(source_path) + ::File.realpath(source_path, path_dir) + rescue + raise SafeZip::Extract::SymlinkSourceDoesNotExistError, "Symlink source #{source_path} does not exist" + end + end +end diff --git a/lib/safe_zip/extract.rb b/lib/safe_zip/extract.rb new file mode 100644 index 00000000000..3bd4935ef34 --- /dev/null +++ b/lib/safe_zip/extract.rb @@ -0,0 +1,73 @@ +# frozen_string_literal: true + +module SafeZip + class Extract + Error = Class.new(StandardError) + PermissionDeniedError = Class.new(Error) + SymlinkSourceDoesNotExistError = Class.new(Error) + UnsupportedEntryError = Class.new(Error) + AlreadyExistsError = Class.new(Error) + NoMatchingError = Class.new(Error) + ExtractError = Class.new(Error) + + attr_reader :archive_path + + def initialize(archive_file) + @archive_path = archive_file + end + + def extract(opts = {}) + params = SafeZip::ExtractParams.new(**opts) + + if Feature.enabled?(:safezip_use_rubyzip, default_enabled: true) + extract_with_ruby_zip(params) + else + legacy_unsafe_extract_with_system_zip(params) + end + end + + private + + def extract_with_ruby_zip(params) + Zip::File.open(archive_path) do |zip_archive| + # Extract all files in the following order: + # 1. Directories first, + # 2. Files next, + # 3. Symlinks last (or anything else) + extracted = extract_all_entries(zip_archive, params, + zip_archive.lazy.select(&:directory?)) + + extracted += extract_all_entries(zip_archive, params, + zip_archive.lazy.select(&:file?)) + + extracted += extract_all_entries(zip_archive, params, + zip_archive.lazy.reject(&:directory?).reject(&:file?)) + + raise NoMatchingError, 'No entries extracted' unless extracted > 0 + end + end + + def extract_all_entries(zip_archive, params, entries) + entries.count do |zip_entry| + SafeZip::Entry.new(zip_archive, zip_entry, params) + .extract + end + end + + def legacy_unsafe_extract_with_system_zip(params) + # Requires UnZip at least 6.00 Info-ZIP. + # -n never overwrite existing files + args = %W(unzip -n -qq #{archive_path}) + + # We add * to end of directory, because we want to extract directory and all subdirectories + args += params.directories_wildcard + + # Target directory where we extract + args += %W(-d #{params.extract_path}) + + unless system(*args) + raise Error, 'archive failed to extract' + end + end + end +end diff --git a/lib/safe_zip/extract_params.rb b/lib/safe_zip/extract_params.rb new file mode 100644 index 00000000000..bd3b788bac9 --- /dev/null +++ b/lib/safe_zip/extract_params.rb @@ -0,0 +1,36 @@ +# frozen_string_literal: true + +module SafeZip + class ExtractParams + include Gitlab::Utils::StrongMemoize + + attr_reader :directories, :extract_path + + def initialize(directories:, to:) + @directories = directories + @extract_path = ::File.realpath(to) + end + + def matching_target_directory(path) + target_directories.find do |directory| + path.start_with?(directory) + end + end + + def target_directories + strong_memoize(:target_directories) do + directories.map do |directory| + ::File.join(::File.expand_path(directory, extract_path), '') + end + end + end + + def directories_wildcard + strong_memoize(:directories_wildcard) do + directories.map do |directory| + ::File.join(directory, '*') + end + end + end + end +end |