From e92925533667e147ff34cf1e9b8af21680c8c7d4 Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Mon, 30 Jan 2023 09:13:00 +0000 Subject: Add latest changes from gitlab-org/security/gitlab@15-8-stable-ee --- lib/gitlab/ci/artifact_file_reader.rb | 23 ++++++++++++----------- lib/gitlab/utils.rb | 3 ++- lib/safe_zip/entry.rb | 10 ++++++++-- lib/safe_zip/extract.rb | 1 + lib/safe_zip/extract_params.rb | 24 ++++++++++++++++++++++-- 5 files changed, 45 insertions(+), 16 deletions(-) (limited to 'lib') diff --git a/lib/gitlab/ci/artifact_file_reader.rb b/lib/gitlab/ci/artifact_file_reader.rb index b0fad026ec5..2eb8df01d58 100644 --- a/lib/gitlab/ci/artifact_file_reader.rb +++ b/lib/gitlab/ci/artifact_file_reader.rb @@ -9,6 +9,7 @@ module Gitlab Error = Class.new(StandardError) MAX_ARCHIVE_SIZE = 5.megabytes + TMP_ARTIFACT_EXTRACTION_DIR = "extracted_artifacts_job_%{id}" def initialize(job) @job = job @@ -45,20 +46,20 @@ module Gitlab end def read_zip_file!(file_path) - job.artifacts_file.use_open_file do |file| - zip_file = Zip::File.new(file, false, true) - entry = zip_file.find_entry(file_path) + dir_name = format(TMP_ARTIFACT_EXTRACTION_DIR, id: job.id.to_i) - unless entry - raise Error, "Path `#{file_path}` does not exist inside the `#{job.name}` artifacts archive!" + job.artifacts_file.use_open_file(unlink_early: false) do |tmp_open_file| + Dir.mktmpdir(dir_name) do |tmp_dir| + SafeZip::Extract.new(tmp_open_file.file_path).extract(files: [file_path], to: tmp_dir) + File.read(File.join(tmp_dir, file_path)) end - - if entry.name_is_directory? - raise Error, "Path `#{file_path}` was expected to be a file but it was a directory!" - end - - zip_file.read(entry) end + rescue SafeZip::Extract::NoMatchingError + raise Error, "Path `#{file_path}` does not exist inside the `#{job.name}` artifacts archive!" + rescue SafeZip::Extract::EntrySizeError + raise Error, "Path `#{file_path}` has invalid size in the zip!" + rescue Errno::EISDIR + raise Error, "Path `#{file_path}` was expected to be a file but it was a directory!" end def max_archive_size_in_mb diff --git a/lib/gitlab/utils.rb b/lib/gitlab/utils.rb index d3055569ece..8bd4cd2401d 100644 --- a/lib/gitlab/utils.rb +++ b/lib/gitlab/utils.rb @@ -4,6 +4,7 @@ module Gitlab module Utils extend self PathTraversalAttackError ||= Class.new(StandardError) + DoubleEncodingError ||= Class.new(StandardError) private_class_method def logger @logger ||= Gitlab::AppLogger @@ -55,7 +56,7 @@ module Gitlab def decode_path(encoded_path) decoded = CGI.unescape(encoded_path) if decoded != CGI.unescape(decoded) - raise StandardError, "path #{encoded_path} is not allowed" + raise DoubleEncodingError, "path #{encoded_path} is not allowed" end decoded diff --git a/lib/safe_zip/entry.rb b/lib/safe_zip/entry.rb index 52d70e83154..88647b9b1eb 100644 --- a/lib/safe_zip/entry.rb +++ b/lib/safe_zip/entry.rb @@ -25,8 +25,8 @@ module SafeZip end def extract - # do not extract if file is not part of target directory - return false unless matching_target_directory + # do not extract if file is not part of target directory or target file + return false unless matching_target_directory || matching_target_file # do not overwrite existing file raise SafeZip::Extract::AlreadyExistsError, "File already exists #{zip_entry.name}" if exist? @@ -44,6 +44,8 @@ module SafeZip end rescue SafeZip::Extract::Error raise + rescue Zip::EntrySizeError => e + raise SafeZip::Extract::EntrySizeError, e.message rescue StandardError => e raise SafeZip::Extract::ExtractError, e.message end @@ -84,6 +86,10 @@ module SafeZip params.matching_target_directory(path) end + def matching_target_file + params.matching_target_file(path) + end + def read_symlink zip_archive.read(zip_entry) end diff --git a/lib/safe_zip/extract.rb b/lib/safe_zip/extract.rb index 74df7895afe..b86941e6bea 100644 --- a/lib/safe_zip/extract.rb +++ b/lib/safe_zip/extract.rb @@ -6,6 +6,7 @@ module SafeZip PermissionDeniedError = Class.new(Error) SymlinkSourceDoesNotExistError = Class.new(Error) UnsupportedEntryError = Class.new(Error) + EntrySizeError = Class.new(Error) AlreadyExistsError = Class.new(Error) NoMatchingError = Class.new(Error) ExtractError = Class.new(Error) diff --git a/lib/safe_zip/extract_params.rb b/lib/safe_zip/extract_params.rb index bd3b788bac9..96881ad1abc 100644 --- a/lib/safe_zip/extract_params.rb +++ b/lib/safe_zip/extract_params.rb @@ -4,11 +4,13 @@ module SafeZip class ExtractParams include Gitlab::Utils::StrongMemoize - attr_reader :directories, :extract_path + attr_reader :directories, :files, :extract_path - def initialize(directories:, to:) + def initialize(to:, directories: [], files: []) @directories = directories + @files = files @extract_path = ::File.realpath(to) + validate! end def matching_target_directory(path) @@ -32,5 +34,23 @@ module SafeZip end end end + + def matching_target_file(path) + target_files.include?(path) + end + + private + + def target_files + strong_memoize(:target_files) do + files.map do |file| + ::File.join(extract_path, file) + end + end + end + + def validate! + raise ArgumentError, 'Either directories or files are required' if directories.empty? && files.empty? + end end end -- cgit v1.2.3