diff options
Diffstat (limited to 'lib')
-rw-r--r-- | lib/api/ci/secure_files.rb | 2 | ||||
-rw-r--r-- | lib/api/helpers.rb | 4 | ||||
-rw-r--r-- | lib/api/validations/validators/file_path.rb | 2 | ||||
-rw-r--r-- | lib/api/validations/validators/integer_or_custom_value.rb | 2 | ||||
-rw-r--r-- | lib/bulk_imports/common/pipelines/lfs_objects_pipeline.rb | 4 | ||||
-rw-r--r-- | lib/bulk_imports/common/pipelines/uploads_pipeline.rb | 2 | ||||
-rw-r--r-- | lib/bulk_imports/file_downloads/validations.rb | 2 | ||||
-rw-r--r-- | lib/bulk_imports/projects/pipelines/design_bundle_pipeline.rb | 4 | ||||
-rw-r--r-- | lib/bulk_imports/projects/pipelines/repository_bundle_pipeline.rb | 4 | ||||
-rw-r--r-- | lib/gitlab/import_export/decompressed_archive_size_validator.rb | 2 | ||||
-rw-r--r-- | lib/gitlab/import_export/project/exported_relations_merger.rb | 4 | ||||
-rw-r--r-- | lib/gitlab/import_export/recursive_merge_folders.rb | 6 | ||||
-rw-r--r-- | lib/gitlab/path_traversal.rb | 47 | ||||
-rw-r--r-- | lib/gitlab/sentence.rb | 12 | ||||
-rw-r--r-- | lib/gitlab/template/finders/global_template_finder.rb | 2 | ||||
-rw-r--r-- | lib/gitlab/template/finders/repo_template_finder.rb | 2 | ||||
-rw-r--r-- | lib/gitlab/utils.rb | 46 |
17 files changed, 82 insertions, 65 deletions
diff --git a/lib/api/ci/secure_files.rb b/lib/api/ci/secure_files.rb index 41faaf80c82..c08889b87a2 100644 --- a/lib/api/ci/secure_files.rb +++ b/lib/api/ci/secure_files.rb @@ -81,7 +81,7 @@ module API route_setting :authentication, basic_auth_personal_access_token: true, job_token_allowed: true post ':id/secure_files' do secure_file = user_project.secure_files.new( - name: Gitlab::Utils.check_path_traversal!(params[:name]) + name: Gitlab::PathTraversal.check_path_traversal!(params[:name]) ) secure_file.file = params[:file] diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index 98f6733a6f2..df080c8e666 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -20,6 +20,10 @@ module API API_RESPONSE_STATUS_CODE = 'gitlab.api.response_status_code' INTEGER_ID_REGEX = /^-?\d+$/.freeze + def logger + API.logger + end + def declared_params(options = {}) options = { include_parent_namespaces: false }.merge(options) declared(params, options).to_h.symbolize_keys diff --git a/lib/api/validations/validators/file_path.rb b/lib/api/validations/validators/file_path.rb index 268ddc29d4e..2440d8890e2 100644 --- a/lib/api/validations/validators/file_path.rb +++ b/lib/api/validations/validators/file_path.rb @@ -8,7 +8,7 @@ module API options = @option.is_a?(Hash) ? @option : {} path_allowlist = options.fetch(:allowlist, []) path = params[attr_name] - Gitlab::Utils.check_allowed_absolute_path_and_path_traversal!(path, path_allowlist) + Gitlab::PathTraversal.check_allowed_absolute_path_and_path_traversal!(path, path_allowlist) rescue StandardError raise Grape::Exceptions::Validation.new( params: [@scope.full_name(attr_name)], diff --git a/lib/api/validations/validators/integer_or_custom_value.rb b/lib/api/validations/validators/integer_or_custom_value.rb index d2352495948..ad7848583fc 100644 --- a/lib/api/validations/validators/integer_or_custom_value.rb +++ b/lib/api/validations/validators/integer_or_custom_value.rb @@ -15,7 +15,7 @@ module API return if value.is_a?(Integer) return if @custom_values.map(&:downcase).include?(value.to_s.downcase) - valid_options = Gitlab::Utils.to_exclusive_sentence(['an integer'] + @custom_values) + valid_options = Gitlab::Sentence.to_exclusive_sentence(['an integer'] + @custom_values) raise Grape::Exceptions::Validation.new( params: [@scope.full_name(attr_name)], message: "should be #{valid_options}, however got #{value}" diff --git a/lib/bulk_imports/common/pipelines/lfs_objects_pipeline.rb b/lib/bulk_imports/common/pipelines/lfs_objects_pipeline.rb index 2e6a29f4738..68bd64dc2ff 100644 --- a/lib/bulk_imports/common/pipelines/lfs_objects_pipeline.rb +++ b/lib/bulk_imports/common/pipelines/lfs_objects_pipeline.rb @@ -18,8 +18,8 @@ module BulkImports # rubocop: disable CodeReuse/ActiveRecord def load(_context, file_path) - Gitlab::Utils.check_path_traversal!(file_path) - Gitlab::Utils.check_allowed_absolute_path!(file_path, [Dir.tmpdir]) + Gitlab::PathTraversal.check_path_traversal!(file_path) + Gitlab::PathTraversal.check_allowed_absolute_path!(file_path, [Dir.tmpdir]) return if tar_filepath?(file_path) return if lfs_json_filepath?(file_path) diff --git a/lib/bulk_imports/common/pipelines/uploads_pipeline.rb b/lib/bulk_imports/common/pipelines/uploads_pipeline.rb index a1b338aeb9f..06132791ea6 100644 --- a/lib/bulk_imports/common/pipelines/uploads_pipeline.rb +++ b/lib/bulk_imports/common/pipelines/uploads_pipeline.rb @@ -22,7 +22,7 @@ module BulkImports def load(context, file_path) # Validate that the path is OK to load - Gitlab::Utils.check_allowed_absolute_path_and_path_traversal!(file_path, [Dir.tmpdir]) + Gitlab::PathTraversal.check_allowed_absolute_path_and_path_traversal!(file_path, [Dir.tmpdir]) return if File.directory?(file_path) return if File.lstat(file_path).symlink? diff --git a/lib/bulk_imports/file_downloads/validations.rb b/lib/bulk_imports/file_downloads/validations.rb index ae94267a6e8..b852a50c888 100644 --- a/lib/bulk_imports/file_downloads/validations.rb +++ b/lib/bulk_imports/file_downloads/validations.rb @@ -22,7 +22,7 @@ module BulkImports private def validate_filepath - Gitlab::Utils.check_path_traversal!(filepath) + Gitlab::PathTraversal.check_path_traversal!(filepath) end def validate_content_type diff --git a/lib/bulk_imports/projects/pipelines/design_bundle_pipeline.rb b/lib/bulk_imports/projects/pipelines/design_bundle_pipeline.rb index 2d5231b0541..373cd2bd75a 100644 --- a/lib/bulk_imports/projects/pipelines/design_bundle_pipeline.rb +++ b/lib/bulk_imports/projects/pipelines/design_bundle_pipeline.rb @@ -20,8 +20,8 @@ module BulkImports end def load(_context, bundle_path) - Gitlab::Utils.check_path_traversal!(bundle_path) - Gitlab::Utils.check_allowed_absolute_path!(bundle_path, [Dir.tmpdir]) + Gitlab::PathTraversal.check_path_traversal!(bundle_path) + Gitlab::PathTraversal.check_allowed_absolute_path!(bundle_path, [Dir.tmpdir]) return unless portable.lfs_enabled? return unless File.exist?(bundle_path) diff --git a/lib/bulk_imports/projects/pipelines/repository_bundle_pipeline.rb b/lib/bulk_imports/projects/pipelines/repository_bundle_pipeline.rb index 9a3c582642f..f19d8931f4a 100644 --- a/lib/bulk_imports/projects/pipelines/repository_bundle_pipeline.rb +++ b/lib/bulk_imports/projects/pipelines/repository_bundle_pipeline.rb @@ -21,8 +21,8 @@ module BulkImports end def load(_context, bundle_path) - Gitlab::Utils.check_path_traversal!(bundle_path) - Gitlab::Utils.check_allowed_absolute_path!(bundle_path, [Dir.tmpdir]) + Gitlab::PathTraversal.check_path_traversal!(bundle_path) + Gitlab::PathTraversal.check_allowed_absolute_path!(bundle_path, [Dir.tmpdir]) return unless File.exist?(bundle_path) return if File.directory?(bundle_path) diff --git a/lib/gitlab/import_export/decompressed_archive_size_validator.rb b/lib/gitlab/import_export/decompressed_archive_size_validator.rb index 564008e7a73..104c9e6c456 100644 --- a/lib/gitlab/import_export/decompressed_archive_size_validator.rb +++ b/lib/gitlab/import_export/decompressed_archive_size_validator.rb @@ -85,7 +85,7 @@ module Gitlab end def validate_archive_path - Gitlab::Utils.check_path_traversal!(@archive_path) + Gitlab::PathTraversal.check_path_traversal!(@archive_path) raise(ServiceError, 'Archive path is a symlink') if File.lstat(@archive_path).symlink? raise(ServiceError, 'Archive path is not a file') unless File.file?(@archive_path) diff --git a/lib/gitlab/import_export/project/exported_relations_merger.rb b/lib/gitlab/import_export/project/exported_relations_merger.rb index dda3d00d608..b5eba768e56 100644 --- a/lib/gitlab/import_export/project/exported_relations_merger.rb +++ b/lib/gitlab/import_export/project/exported_relations_merger.rb @@ -20,8 +20,8 @@ module Gitlab tar_gz_full_path = File.join(dirpath, filename) decompress_path = File.join(dirpath, relation) - Gitlab::Utils.check_path_traversal!(tar_gz_full_path) - Gitlab::Utils.check_path_traversal!(decompress_path) + Gitlab::PathTraversal.check_path_traversal!(tar_gz_full_path) + Gitlab::PathTraversal.check_path_traversal!(decompress_path) # Download tar.gz download_or_copy_upload( diff --git a/lib/gitlab/import_export/recursive_merge_folders.rb b/lib/gitlab/import_export/recursive_merge_folders.rb index 982358699bd..827385d4daf 100644 --- a/lib/gitlab/import_export/recursive_merge_folders.rb +++ b/lib/gitlab/import_export/recursive_merge_folders.rb @@ -45,9 +45,9 @@ module Gitlab DEFAULT_DIR_MODE = 0o700 def self.merge(source_path, target_path) - Gitlab::Utils.check_path_traversal!(source_path) - Gitlab::Utils.check_path_traversal!(target_path) - Gitlab::Utils.check_allowed_absolute_path!(source_path, [Dir.tmpdir]) + Gitlab::PathTraversal.check_path_traversal!(source_path) + Gitlab::PathTraversal.check_path_traversal!(target_path) + Gitlab::PathTraversal.check_allowed_absolute_path!(source_path, [Dir.tmpdir]) recursive_merge(source_path, target_path) end diff --git a/lib/gitlab/path_traversal.rb b/lib/gitlab/path_traversal.rb new file mode 100644 index 00000000000..7b8afa79d3b --- /dev/null +++ b/lib/gitlab/path_traversal.rb @@ -0,0 +1,47 @@ +# frozen_string_literal: true + +module Gitlab + module PathTraversal + extend self + PathTraversalAttackError = Class.new(StandardError) + + private_class_method def logger + @logger ||= Gitlab::AppLogger + end + + # Ensure that the relative path will not traverse outside the base directory + # We url decode the path to avoid passing invalid paths forward in url encoded format. + # Also see https://gitlab.com/gitlab-org/gitlab/-/merge_requests/24223#note_284122580 + # It also checks for ALT_SEPARATOR aka '\' (forward slash) + def check_path_traversal!(path) + return unless path + + path = path.to_s if path.is_a?(Gitlab::HashedPath) + raise PathTraversalAttackError, 'Invalid path' unless path.is_a?(String) + + path = ::Gitlab::Utils.decode_path(path) + path_regex = %r{(\A(\.{1,2})\z|\A\.\.[/\\]|[/\\]\.\.\z|[/\\]\.\.[/\\]|\n)} + + if path.match?(path_regex) + logger.warn(message: "Potential path traversal attempt detected", path: path.to_s) + raise PathTraversalAttackError, 'Invalid path' + end + + path + end + + def check_allowed_absolute_path!(path, allowlist) + return unless Pathname.new(path).absolute? + return if ::Gitlab::Utils.allowlisted?(path, allowlist) + + raise StandardError, "path #{path} is not allowed" + end + + def check_allowed_absolute_path_and_path_traversal!(path, path_allowlist) + traversal_path = check_path_traversal!(path) + raise StandardError, "path is not a string!" unless traversal_path.is_a?(String) + + check_allowed_absolute_path!(traversal_path, path_allowlist) + end + end +end diff --git a/lib/gitlab/sentence.rb b/lib/gitlab/sentence.rb new file mode 100644 index 00000000000..963459e31a3 --- /dev/null +++ b/lib/gitlab/sentence.rb @@ -0,0 +1,12 @@ +# frozen_string_literal: true + +module Gitlab + module Sentence + extend self + # Wraps ActiveSupport's Array#to_sentence to convert the given array to a + # comma-separated sentence joined with localized 'or' Strings instead of 'and'. + def to_exclusive_sentence(array) + array.to_sentence(two_words_connector: _(' or '), last_word_connector: _(', or ')) + end + end +end diff --git a/lib/gitlab/template/finders/global_template_finder.rb b/lib/gitlab/template/finders/global_template_finder.rb index 6d2677175e6..a6ab36e6cd2 100644 --- a/lib/gitlab/template/finders/global_template_finder.rb +++ b/lib/gitlab/template/finders/global_template_finder.rb @@ -25,7 +25,7 @@ module Gitlab # The key is untrusted input, so ensure we can't be directed outside # of base_dir - Gitlab::Utils.check_path_traversal!(file_name) + Gitlab::PathTraversal.check_path_traversal!(file_name) directory = select_directory(file_name) directory ? File.join(category_directory(directory), file_name) : nil diff --git a/lib/gitlab/template/finders/repo_template_finder.rb b/lib/gitlab/template/finders/repo_template_finder.rb index 9f0ba97bcdf..8343750e04a 100644 --- a/lib/gitlab/template/finders/repo_template_finder.rb +++ b/lib/gitlab/template/finders/repo_template_finder.rb @@ -29,7 +29,7 @@ module Gitlab # The key is untrusted input, so ensure we can't be directed outside # of base_dir inside the repository - Gitlab::Utils.check_path_traversal!(file_name) + Gitlab::PathTraversal.check_path_traversal!(file_name) directory = select_directory(file_name) raise FileNotFoundError if directory.nil? diff --git a/lib/gitlab/utils.rb b/lib/gitlab/utils.rb index b92e7dbb725..dc0112c14d6 100644 --- a/lib/gitlab/utils.rb +++ b/lib/gitlab/utils.rb @@ -3,34 +3,8 @@ module Gitlab module Utils extend self - PathTraversalAttackError ||= Class.new(StandardError) DoubleEncodingError ||= Class.new(StandardError) - private_class_method def logger - @logger ||= Gitlab::AppLogger - end - - # Ensure that the relative path will not traverse outside the base directory - # We url decode the path to avoid passing invalid paths forward in url encoded format. - # Also see https://gitlab.com/gitlab-org/gitlab/-/merge_requests/24223#note_284122580 - # It also checks for ALT_SEPARATOR aka '\' (forward slash) - def check_path_traversal!(path) - return unless path - - path = path.to_s if path.is_a?(Gitlab::HashedPath) - raise PathTraversalAttackError, 'Invalid path' unless path.is_a?(String) - - path = decode_path(path) - path_regex = %r{(\A(\.{1,2})\z|\A\.\.[/\\]|[/\\]\.\.\z|[/\\]\.\.[/\\]|\n)} - - if path.match?(path_regex) - logger.warn(message: "Potential path traversal attempt detected", path: "#{path}") - raise PathTraversalAttackError, 'Invalid path' - end - - path - end - def allowlisted?(absolute_path, allowlist) path = absolute_path.downcase @@ -39,20 +13,6 @@ module Gitlab end end - def check_allowed_absolute_path!(path, allowlist) - return unless Pathname.new(path).absolute? - return if allowlisted?(path, allowlist) - - raise StandardError, "path #{path} is not allowed" - end - - def check_allowed_absolute_path_and_path_traversal!(path, path_allowlist) - traversal_path = check_path_traversal!(path) - raise StandardError, "path is not a string!" unless traversal_path.is_a?(String) - - check_allowed_absolute_path!(traversal_path, path_allowlist) - end - def decode_path(encoded_path) decoded = CGI.unescape(encoded_path) if decoded != CGI.unescape(decoded) @@ -103,12 +63,6 @@ module Gitlab .gsub(/(\A-+|-+\z)/, '') end - # Wraps ActiveSupport's Array#to_sentence to convert the given array to a - # comma-separated sentence joined with localized 'or' Strings instead of 'and'. - def to_exclusive_sentence(array) - array.to_sentence(two_words_connector: _(' or '), last_word_connector: _(', or ')) - end - # Converts newlines into HTML line break elements def nlbr(str) ActionView::Base.full_sanitizer.sanitize(+str, tags: []).gsub(/\r?\n/, '<br>').html_safe |