diff --git a/app/controllers/concerns/uploads_actions.rb b/app/controllers/concerns/uploads_actions.rb
index 0d64a685065726e10def6d80c8f89c1b57dea2e0..d747d397e20dcb4dcbed809749a9810e4cec7e8c 100644
--- a/app/controllers/concerns/uploads_actions.rb
+++ b/app/controllers/concerns/uploads_actions.rb
@@ -11,7 +11,7 @@ module UploadsActions
prepend_before_action :set_request_format_from_path_extension
rescue_from FileUploader::InvalidSecret, with: :render_404
- rescue_from ::Gitlab::Utils::PathTraversalAttackError do
+ rescue_from ::Gitlab::PathTraversal::PathTraversalAttackError do
head :bad_request
end
end
@@ -37,7 +37,7 @@ def create
# - or redirect to its URL
#
def show
- Gitlab::Utils.check_path_traversal!(params[:filename])
+ Gitlab::PathTraversal.check_path_traversal!(params[:filename])
return render_404 unless uploader&.exists?
diff --git a/app/controllers/groups/dependency_proxy_for_containers_controller.rb b/app/controllers/groups/dependency_proxy_for_containers_controller.rb
index 1b1aed0ec2ed3e4fd0b51deca6e86f1b66066bdf..1fc631f299bad6d734d4e564973eecabb04577cc 100644
--- a/app/controllers/groups/dependency_proxy_for_containers_controller.rb
+++ b/app/controllers/groups/dependency_proxy_for_containers_controller.rb
@@ -121,7 +121,7 @@ def blob_file_name
end
def manifest_file_name
- @manifest_file_name ||= Gitlab::Utils.check_path_traversal!("#{image}:#{tag}.json")
+ @manifest_file_name ||= Gitlab::PathTraversal.check_path_traversal!("#{image}:#{tag}.json")
end
def group
diff --git a/app/controllers/projects/releases_controller.rb b/app/controllers/projects/releases_controller.rb
index 7c569df7267521c0d322933b7c810f506e0bf371..6a6a47bc33d948a551b048e0951f8d2f5c0d5135 100644
--- a/app/controllers/projects/releases_controller.rb
+++ b/app/controllers/projects/releases_controller.rb
@@ -74,6 +74,6 @@ def fetch_latest_tag
end
def validate_suffix_path
- Gitlab::Utils.check_path_traversal!(params[:suffix_path]) if params[:suffix_path]
+ Gitlab::PathTraversal.check_path_traversal!(params[:suffix_path]) if params[:suffix_path]
end
end
diff --git a/app/finders/uploader_finder.rb b/app/finders/uploader_finder.rb
index 0d1de0d56fd862d039c6c11a02a5cab542a78ccf..e4a0e831720ca3fc9c58622be95267a058a5e9aa 100644
--- a/app/finders/uploader_finder.rb
+++ b/app/finders/uploader_finder.rb
@@ -16,12 +16,12 @@ def execute
retrieve_file_state!
uploader
- rescue ::Gitlab::Utils::PathTraversalAttackError
+ rescue ::Gitlab::PathTraversal::PathTraversalAttackError
nil # no-op if for incorrect files
end
def prevent_path_traversal_attack!
- Gitlab::Utils.check_path_traversal!(@file_path)
+ Gitlab::PathTraversal.check_path_traversal!(@file_path)
end
def retrieve_file_state!
diff --git a/app/models/concerns/sanitizable.rb b/app/models/concerns/sanitizable.rb
index 653d7a4875dbbbf1c45b445c2391a11cf426a8c6..d05ce389ebfe61ecb6f71a278aeef86fe984a6f6 100644
--- a/app/models/concerns/sanitizable.rb
+++ b/app/models/concerns/sanitizable.rb
@@ -48,11 +48,11 @@ def sanitizes!(*attrs)
# This method raises an exception on failure so perform this
# last if multiple errors should be returned.
- Gitlab::Utils.check_path_traversal!(input.to_s)
+ Gitlab::PathTraversal.check_path_traversal!(input.to_s)
rescue Gitlab::Utils::DoubleEncodingError
record.errors.add(attr, 'cannot contain escaped components')
- rescue Gitlab::Utils::PathTraversalAttackError
+ rescue Gitlab::PathTraversal::PathTraversalAttackError
record.errors.add(attr, "cannot contain a path traversal component")
end
end
diff --git a/app/models/merge_request_diff.rb b/app/models/merge_request_diff.rb
index d36857fc94a8933e32dbd6970d1893c04f0d54b0..33930836c48fc7c0b3447414178575a6cc7b857c 100644
--- a/app/models/merge_request_diff.rb
+++ b/app/models/merge_request_diff.rb
@@ -592,8 +592,8 @@ def cached_external_diff
end
def remove_cached_external_diff
- Gitlab::Utils.check_path_traversal!(external_diff_cache_dir)
- Gitlab::Utils.check_allowed_absolute_path!(external_diff_cache_dir, [Dir.tmpdir])
+ Gitlab::PathTraversal.check_path_traversal!(external_diff_cache_dir)
+ Gitlab::PathTraversal.check_allowed_absolute_path!(external_diff_cache_dir, [Dir.tmpdir])
return unless Dir.exist?(external_diff_cache_dir)
diff --git a/app/serializers/integrations/harbor_serializers/repository_entity.rb b/app/serializers/integrations/harbor_serializers/repository_entity.rb
index f03465fe8e2c09742a4abaafac3727c007bb7d67..a6366ebfb36fd67cc136bb8994471cedee471b51 100644
--- a/app/serializers/integrations/harbor_serializers/repository_entity.rb
+++ b/app/serializers/integrations/harbor_serializers/repository_entity.rb
@@ -47,8 +47,8 @@ class RepositoryEntity < Grape::Entity
private
def validate_path(path)
- Gitlab::Utils.check_path_traversal!(path)
- rescue ::Gitlab::Utils::PathTraversalAttackError
+ Gitlab::PathTraversal.check_path_traversal!(path)
+ rescue ::Gitlab::PathTraversal::PathTraversalAttackError
Gitlab::AppLogger.error("Path traversal attack detected #{path}")
''
end
diff --git a/app/services/bulk_imports/archive_extraction_service.rb b/app/services/bulk_imports/archive_extraction_service.rb
index fec8fd0e1f56364211d49d78c5e6681b26e1d9cf..4485b19035b83ad4de0fd3db07cfc43927456743 100644
--- a/app/services/bulk_imports/archive_extraction_service.rb
+++ b/app/services/bulk_imports/archive_extraction_service.rb
@@ -41,11 +41,11 @@ def execute
attr_reader :tmpdir, :filename, :filepath
def validate_filepath
- Gitlab::Utils.check_path_traversal!(filepath)
+ Gitlab::PathTraversal.check_path_traversal!(filepath)
end
def validate_tmpdir
- Gitlab::Utils.check_allowed_absolute_path!(tmpdir, [Dir.tmpdir])
+ Gitlab::PathTraversal.check_allowed_absolute_path!(tmpdir, [Dir.tmpdir])
end
def validate_symlink
diff --git a/app/services/bulk_imports/file_decompression_service.rb b/app/services/bulk_imports/file_decompression_service.rb
index 41616fc1c7579bcd7a83dd090cfee3e465e2f610..94573f6bb13cb455da72216fdefdcff30c832e60 100644
--- a/app/services/bulk_imports/file_decompression_service.rb
+++ b/app/services/bulk_imports/file_decompression_service.rb
@@ -41,11 +41,11 @@ def execute
attr_reader :tmpdir, :filename, :filepath, :decompressed_filename, :decompressed_filepath
def validate_filepath
- Gitlab::Utils.check_path_traversal!(filepath)
+ Gitlab::PathTraversal.check_path_traversal!(filepath)
end
def validate_tmpdir
- Gitlab::Utils.check_allowed_absolute_path!(tmpdir, [Dir.tmpdir])
+ Gitlab::PathTraversal.check_allowed_absolute_path!(tmpdir, [Dir.tmpdir])
end
def validate_decompressed_file_size
diff --git a/app/services/bulk_imports/file_download_service.rb b/app/services/bulk_imports/file_download_service.rb
index ee499c782b45fb7fc131d5e074eeca922c7534ed..ef7e0ae825848877816b1a6f61c989d7c8d9e230 100644
--- a/app/services/bulk_imports/file_download_service.rb
+++ b/app/services/bulk_imports/file_download_service.rb
@@ -99,7 +99,7 @@ def allow_local_requests?
end
def validate_tmpdir
- Gitlab::Utils.check_allowed_absolute_path!(tmpdir, [Dir.tmpdir])
+ Gitlab::PathTraversal.check_allowed_absolute_path!(tmpdir, [Dir.tmpdir])
end
def filepath
diff --git a/app/services/projects/readme_renderer_service.rb b/app/services/projects/readme_renderer_service.rb
index 6871976aded2c53276764b1b3f5c55b33b2c96ab..8fd33a717c5ebd28bd6d149a21df8fd7e28ca0e4 100644
--- a/app/services/projects/readme_renderer_service.rb
+++ b/app/services/projects/readme_renderer_service.rb
@@ -17,9 +17,9 @@ def render(template_name)
end
def sanitized_filename(template_name)
- path = Gitlab::Utils.check_path_traversal!("#{template_name}.md.tt")
+ path = Gitlab::PathTraversal.check_path_traversal!("#{template_name}.md.tt")
path = TEMPLATE_PATH.join(path).to_s
- Gitlab::Utils.check_allowed_absolute_path!(path, [TEMPLATE_PATH.to_s])
+ Gitlab::PathTraversal.check_allowed_absolute_path!(path, [TEMPLATE_PATH.to_s])
path
end
diff --git a/app/uploaders/gitlab_uploader.rb b/app/uploaders/gitlab_uploader.rb
index 2eb34288bd71be1f173369c3e510c54665fb5fdd..06bf742a22d29f1e25953cc9f222a58365d73d76 100644
--- a/app/uploaders/gitlab_uploader.rb
+++ b/app/uploaders/gitlab_uploader.rb
@@ -189,10 +189,10 @@ def pathname
#
# @param [CarrierWave::SanitizedFile]
# @return [Nil]
- # @raise [Gitlab::Utils::PathTraversalAttackError]
+ # @raise [Gitlab::PathTraversal::PathTraversalAttackError]
def protect_from_path_traversal!(file)
PROTECTED_METHODS.each do |method|
- Gitlab::Utils.check_path_traversal!(self.send(method)) # rubocop: disable GitlabSecurity/PublicSend
+ Gitlab::PathTraversal.check_path_traversal!(self.send(method)) # rubocop: disable GitlabSecurity/PublicSend
rescue ObjectNotReadyError
# Do nothing. This test was attempted before the file was ready for that method
diff --git a/app/uploaders/object_storage.rb b/app/uploaders/object_storage.rb
index 70f7625ffbd91b3f3ae9efec86df9e0e8bf54857..672433ec534bb3526fe11bac4795f183bac9a437 100644
--- a/app/uploaders/object_storage.rb
+++ b/app/uploaders/object_storage.rb
@@ -426,7 +426,7 @@ def delete_tmp_file_after_storage
end
def retrieve_from_store!(identifier)
- Gitlab::Utils.check_path_traversal!(identifier)
+ Gitlab::PathTraversal.check_path_traversal!(identifier)
# We need to force assign the value of @filename so that we will still
# get the original_filename in cases wherein the file points to a random generated
diff --git a/doc/development/secure_coding_guidelines.md b/doc/development/secure_coding_guidelines.md
index e8fda066ca3a3f149acf0b322999044e124fa6da..c5e7a58af0d8bbf1425240c4d8f0fc5d195d5984 100644
--- a/doc/development/secure_coding_guidelines.md
+++ b/doc/development/secure_coding_guidelines.md
@@ -485,7 +485,7 @@ In order to prevent Path Traversal vulnerabilities, user-controlled filenames or
#### GitLab specific validations
-The methods `Gitlab::Utils.check_path_traversal!()` and `Gitlab::Utils.check_allowed_absolute_path!()`
+The methods `Gitlab::PathTraversal.check_path_traversal!()` and `Gitlab::PathTraversal.check_allowed_absolute_path!()`
can be used to validate user-supplied paths and prevent vulnerabilities.
`check_path_traversal!()` will detect their Path Traversal payloads and accepts URL-encoded paths.
`check_allowed_absolute_path!()` will check if a path is absolute and whether it is inside the allowed path list. By default, absolute
@@ -495,7 +495,7 @@ parameter when using `check_allowed_absolute_path!()`.
To use a combination of both checks, follow the example below:
```ruby
-Gitlab::Utils.check_allowed_absolute_path_and_path_traversal!(path, path_allowlist)
+Gitlab::PathTraversal.check_allowed_absolute_path_and_path_traversal!(path, path_allowlist)
```
In the REST API, we have the [`FilePath`](https://gitlab.com/gitlab-org/security/gitlab/-/blob/master/lib/api/validations/validators/file_path.rb)
diff --git a/ee/app/models/product_analytics/visualization.rb b/ee/app/models/product_analytics/visualization.rb
index c569b778f7577c2b809a968f415e7d2cbce344f9..cd80541e0ef4237ad841feebb7f4852e208b3d8b 100644
--- a/ee/app/models/product_analytics/visualization.rb
+++ b/ee/app/models/product_analytics/visualization.rb
@@ -13,8 +13,8 @@ def self.from_data(data:, project:)
return new(config: config) if config
file = Rails.root.join('ee/lib/gitlab/analytics/product_analytics/visualizations', "#{data}.yaml")
- Gitlab::Utils.check_path_traversal!(data)
- Gitlab::Utils.check_allowed_absolute_path!(
+ Gitlab::PathTraversal.check_path_traversal!(data)
+ Gitlab::PathTraversal.check_allowed_absolute_path!(
file.to_s, [Rails.root.join('ee/lib/gitlab/analytics/product_analytics/visualizations').to_s]
)
new(config: File.read(file))
diff --git a/ee/spec/models/product_analytics/visualization_spec.rb b/ee/spec/models/product_analytics/visualization_spec.rb
index 08a2feedb365cf643d7e66d503b189dbf21c229f..7f99ff2d2116185ac21e029fc9638e1ea7cd7676 100644
--- a/ee/spec/models/product_analytics/visualization_spec.rb
+++ b/ee/spec/models/product_analytics/visualization_spec.rb
@@ -43,7 +43,7 @@
let(:dashboard) { dashboards.find { |d| d.title == 'Dashboard Example 1' } }
it 'raises an error' do
- expect { dashboard.panels.first.visualization }.to raise_error(Gitlab::Utils::PathTraversalAttackError)
+ expect { dashboard.panels.first.visualization }.to raise_error(Gitlab::PathTraversal::PathTraversalAttackError)
end
end
end
diff --git a/lib/api/ci/secure_files.rb b/lib/api/ci/secure_files.rb
index 41faaf80c82f3b8d1e915c8ac21c35c148dd544f..c08889b87a2bcd41982239413fec075f3f99bc77 100644
--- a/lib/api/ci/secure_files.rb
+++ b/lib/api/ci/secure_files.rb
@@ -81,7 +81,7 @@ class SecureFiles < ::API::Base
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 83641e824ae54354624f3bec1b08b361fbbc6e04..024fbe02d54fd618749a1db22ed585d3ae63a5e7 100644
--- a/lib/api/helpers.rb
+++ b/lib/api/helpers.rb
@@ -20,6 +20,10 @@ module Helpers
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 268ddc29d4e65ae594cca05bd002f6b09d5eda88..2440d8890e2b4e7ab17163985669b10acc8d8264 100644
--- a/lib/api/validations/validators/file_path.rb
+++ b/lib/api/validations/validators/file_path.rb
@@ -8,7 +8,7 @@ def validate_param!(attr_name, params)
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/bulk_imports/common/pipelines/lfs_objects_pipeline.rb b/lib/bulk_imports/common/pipelines/lfs_objects_pipeline.rb
index 2e6a29f4738b5ac169e695238bddc3a2b46272c0..68bd64dc2ff4a0872f0062243d17977ed4742832 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 @@ def extract(_context)
# 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 a1b338aeb9f06404aed1f1d6ea5e996a39729c57..06132791ea60ee0b7a2cb39731c70dc020f0bdb8 100644
--- a/lib/bulk_imports/common/pipelines/uploads_pipeline.rb
+++ b/lib/bulk_imports/common/pipelines/uploads_pipeline.rb
@@ -22,7 +22,7 @@ def extract(_context)
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 ae94267a6e840f8283e10359f2493155cddc1ce7..b852a50c8882b99275093048d58b00a538298a15 100644
--- a/lib/bulk_imports/file_downloads/validations.rb
+++ b/lib/bulk_imports/file_downloads/validations.rb
@@ -22,7 +22,7 @@ def response_headers
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 2d5231b0541eedccee9a32bddeb4b9c03cbb1dc6..373cd2bd75a992f938ff28fc706ca28ea6407e9e 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 @@ def extract(_context)
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 9a3c582642f07dd4e36eed1f27f8ccf610d0f2ee..f19d8931f4af9edbe72373ca7c519cbc2deee259 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 @@ def extract(_context)
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 564008e7a7311ec27a731c24b6707652528eef31..104c9e6c456a358a1f336b66bb83af40db8bd7bf 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 @@ def validate
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 dda3d00d608218a7d92250aa061dca8deb9466cf..b5eba768e560925fdfd19c7427977a69ca0179c6 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 @@ def save
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 982358699bd3a9f7aa27cd301fbb50e54418b552..827385d4dafff721d7433b4b781f35c28a0367ef 100644
--- a/lib/gitlab/import_export/recursive_merge_folders.rb
+++ b/lib/gitlab/import_export/recursive_merge_folders.rb
@@ -45,9 +45,9 @@ class RecursiveMergeFolders
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 0000000000000000000000000000000000000000..7b8afa79d3ba9cafa9185dcdfdf0340eb8d87bfe
--- /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/template/finders/global_template_finder.rb b/lib/gitlab/template/finders/global_template_finder.rb
index 6d2677175e63fdd7be9382afc74e9fa8648aa7fc..a6ab36e6cd223b4d3533951b1eb4e608e31af76d 100644
--- a/lib/gitlab/template/finders/global_template_finder.rb
+++ b/lib/gitlab/template/finders/global_template_finder.rb
@@ -25,7 +25,7 @@ def find(key)
# 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 9f0ba97bcdfcd8ebd51b05cc08344490ee02ef57..8343750e04a6b98519fb0da199dfe39a62249793 100644
--- a/lib/gitlab/template/finders/repo_template_finder.rb
+++ b/lib/gitlab/template/finders/repo_template_finder.rb
@@ -29,7 +29,7 @@ def find(key)
# 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 b92e7dbb725dd2ed56e51d092c0587df35c825e3..f43126074f9d497135d29c1efd8b61df6c90ea46 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 @@ def allowlisted?(absolute_path, allowlist)
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)
diff --git a/spec/controllers/groups/dependency_proxy_for_containers_controller_spec.rb b/spec/controllers/groups/dependency_proxy_for_containers_controller_spec.rb
index a59c90a3cf26385018df3b09f7ddaa6583d39d0e..89a75fb53f21faa5ec285549fbf057f3e0ef6460 100644
--- a/spec/controllers/groups/dependency_proxy_for_containers_controller_spec.rb
+++ b/spec/controllers/groups/dependency_proxy_for_containers_controller_spec.rb
@@ -28,7 +28,7 @@
let(:image) { '../path_traversal' }
it 'raises an error' do
- expect { subject }.to raise_error(Gitlab::Utils::PathTraversalAttackError, 'Invalid path')
+ expect { subject }.to raise_error(Gitlab::PathTraversal::PathTraversalAttackError, 'Invalid path')
end
end
@@ -36,7 +36,7 @@
let(:tag) { 'latest%2f..%2f..%2fpath_traversal' }
it 'raises an error' do
- expect { subject }.to raise_error(Gitlab::Utils::PathTraversalAttackError, 'Invalid path')
+ expect { subject }.to raise_error(Gitlab::PathTraversal::PathTraversalAttackError, 'Invalid path')
end
end
end
diff --git a/spec/controllers/projects/releases_controller_spec.rb b/spec/controllers/projects/releases_controller_spec.rb
index 17bf9308834647022961ae2fdcebb7b8f5d21b3a..35ac7ed0aa48e3470d7d0a033594f0a2a7fa45f3 100644
--- a/spec/controllers/projects/releases_controller_spec.rb
+++ b/spec/controllers/projects/releases_controller_spec.rb
@@ -317,7 +317,7 @@
it 'raises attack error' do
expect do
subject
- end.to raise_error(Gitlab::Utils::PathTraversalAttackError)
+ end.to raise_error(Gitlab::PathTraversal::PathTraversalAttackError)
end
end
diff --git a/spec/lib/bulk_imports/common/pipelines/lfs_objects_pipeline_spec.rb b/spec/lib/bulk_imports/common/pipelines/lfs_objects_pipeline_spec.rb
index 5220b9d37e53448da40989c06f439cc2014db5b0..297ac0ca0bad7161670d796b36d00eb30afc2896 100644
--- a/spec/lib/bulk_imports/common/pipelines/lfs_objects_pipeline_spec.rb
+++ b/spec/lib/bulk_imports/common/pipelines/lfs_objects_pipeline_spec.rb
@@ -105,7 +105,7 @@
context 'when file path is being traversed' do
it 'raises an error' do
- expect { pipeline.load(context, File.join(tmpdir, '..')) }.to raise_error(Gitlab::Utils::PathTraversalAttackError, 'Invalid path')
+ expect { pipeline.load(context, File.join(tmpdir, '..')) }.to raise_error(Gitlab::PathTraversal::PathTraversalAttackError, 'Invalid path')
end
end
diff --git a/spec/lib/bulk_imports/common/pipelines/uploads_pipeline_spec.rb b/spec/lib/bulk_imports/common/pipelines/uploads_pipeline_spec.rb
index d6622785e65696fa524dda0f688f7944013d6663..bc6d36452b4481f778a2a7ae200df399d4774c23 100644
--- a/spec/lib/bulk_imports/common/pipelines/uploads_pipeline_spec.rb
+++ b/spec/lib/bulk_imports/common/pipelines/uploads_pipeline_spec.rb
@@ -128,7 +128,7 @@
context 'when path traverses' do
it 'does not upload the file' do
path_traversal = "#{uploads_dir_path}/avatar/../../../../etc/passwd"
- expect { pipeline.load(context, path_traversal) }.to not_change { portable.uploads.count }.and raise_error(Gitlab::Utils::PathTraversalAttackError)
+ expect { pipeline.load(context, path_traversal) }.to not_change { portable.uploads.count }.and raise_error(Gitlab::PathTraversal::PathTraversalAttackError)
end
end
diff --git a/spec/lib/bulk_imports/projects/pipelines/design_bundle_pipeline_spec.rb b/spec/lib/bulk_imports/projects/pipelines/design_bundle_pipeline_spec.rb
index 6a509ca7f146a422ebcd250c8708ece0dee57585..5b7309b09f580743f103d738bb4819400bee75b9 100644
--- a/spec/lib/bulk_imports/projects/pipelines/design_bundle_pipeline_spec.rb
+++ b/spec/lib/bulk_imports/projects/pipelines/design_bundle_pipeline_spec.rb
@@ -146,7 +146,7 @@
context 'when path is being traversed' do
it 'raises an error' do
expect { pipeline.load(context, File.join(tmpdir, '..')) }
- .to raise_error(Gitlab::Utils::PathTraversalAttackError, 'Invalid path')
+ .to raise_error(Gitlab::PathTraversal::PathTraversalAttackError, 'Invalid path')
end
end
end
diff --git a/spec/lib/bulk_imports/projects/pipelines/repository_bundle_pipeline_spec.rb b/spec/lib/bulk_imports/projects/pipelines/repository_bundle_pipeline_spec.rb
index b8c21feb05d6b50ce2e58eb33079bef904b6a10e..07fafc190269fc600ff812636b537eddd07a5e45 100644
--- a/spec/lib/bulk_imports/projects/pipelines/repository_bundle_pipeline_spec.rb
+++ b/spec/lib/bulk_imports/projects/pipelines/repository_bundle_pipeline_spec.rb
@@ -144,7 +144,7 @@
context 'when path is being traversed' do
it 'raises an error' do
expect { pipeline.load(context, File.join(tmpdir, '..')) }
- .to raise_error(Gitlab::Utils::PathTraversalAttackError, 'Invalid path')
+ .to raise_error(Gitlab::PathTraversal::PathTraversalAttackError, 'Invalid path')
end
end
end
diff --git a/spec/lib/gitlab/github_import/attachments_downloader_spec.rb b/spec/lib/gitlab/github_import/attachments_downloader_spec.rb
index dc9f939a19bcb39473cb09e0d0f3431bab6ae42d..f7781226ecf059621b4cf81255e2965e2284ad7f 100644
--- a/spec/lib/gitlab/github_import/attachments_downloader_spec.rb
+++ b/spec/lib/gitlab/github_import/attachments_downloader_spec.rb
@@ -44,7 +44,7 @@
it 'raises expected exception' do
expect { downloader.perform }.to raise_exception(
- Gitlab::Utils::PathTraversalAttackError,
+ Gitlab::PathTraversal::PathTraversalAttackError,
'Invalid path'
)
end
diff --git a/spec/lib/gitlab/import_export/recursive_merge_folders_spec.rb b/spec/lib/gitlab/import_export/recursive_merge_folders_spec.rb
index 6e5be0b282918ecc04d8e27ac03e24b55988dd0e..cb8ac0884938067280b958a58a30a18af5cdce8e 100644
--- a/spec/lib/gitlab/import_export/recursive_merge_folders_spec.rb
+++ b/spec/lib/gitlab/import_export/recursive_merge_folders_spec.rb
@@ -31,7 +31,7 @@
Dir.mktmpdir do |tmpdir|
expect do
described_class.merge("#{tmpdir}/../", tmpdir)
- end.to raise_error(Gitlab::Utils::PathTraversalAttackError)
+ end.to raise_error(Gitlab::PathTraversal::PathTraversalAttackError)
end
end
@@ -47,7 +47,7 @@
Dir.mktmpdir do |tmpdir|
expect do
described_class.merge(tmpdir, "#{tmpdir}/../")
- end.to raise_error(Gitlab::Utils::PathTraversalAttackError)
+ end.to raise_error(Gitlab::PathTraversal::PathTraversalAttackError)
end
end
end
diff --git a/spec/lib/gitlab/path_traversal_spec.rb b/spec/lib/gitlab/path_traversal_spec.rb
new file mode 100644
index 0000000000000000000000000000000000000000..8ae1330c203000b22e0d17e13e3edfd69242bd2c
--- /dev/null
+++ b/spec/lib/gitlab/path_traversal_spec.rb
@@ -0,0 +1,173 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Gitlab::PathTraversal, feature_category: :shared do
+ using RSpec::Parameterized::TableSyntax
+
+ delegate :check_path_traversal!, :check_allowed_absolute_path!,
+ :check_allowed_absolute_path_and_path_traversal!, to: :described_class
+
+ describe '.check_path_traversal!' do
+ it 'detects path traversal in string without any separators' do
+ expect { check_path_traversal!('.') }.to raise_error(/Invalid path/)
+ expect { check_path_traversal!('..') }.to raise_error(/Invalid path/)
+ end
+
+ it 'detects path traversal at the start of the string' do
+ expect { check_path_traversal!('../foo') }.to raise_error(/Invalid path/)
+ expect { check_path_traversal!('..\\foo') }.to raise_error(/Invalid path/)
+ end
+
+ it 'detects path traversal at the start of the string, even to just the subdirectory' do
+ expect { check_path_traversal!('../') }.to raise_error(/Invalid path/)
+ expect { check_path_traversal!('..\\') }.to raise_error(/Invalid path/)
+ expect { check_path_traversal!('/../') }.to raise_error(/Invalid path/)
+ expect { check_path_traversal!('\\..\\') }.to raise_error(/Invalid path/)
+ end
+
+ it 'detects path traversal in the middle of the string' do
+ expect { check_path_traversal!('foo/../../bar') }.to raise_error(/Invalid path/)
+ expect { check_path_traversal!('foo\\..\\..\\bar') }.to raise_error(/Invalid path/)
+ expect { check_path_traversal!('foo/..\\bar') }.to raise_error(/Invalid path/)
+ expect { check_path_traversal!('foo\\../bar') }.to raise_error(/Invalid path/)
+ expect { check_path_traversal!('foo/..\\..\\..\\..\\../bar') }.to raise_error(/Invalid path/)
+ end
+
+ it 'detects path traversal at the end of the string when slash-terminates' do
+ expect { check_path_traversal!('foo/../') }.to raise_error(/Invalid path/)
+ expect { check_path_traversal!('foo\\..\\') }.to raise_error(/Invalid path/)
+ end
+
+ it 'detects path traversal at the end of the string' do
+ expect { check_path_traversal!('foo/..') }.to raise_error(/Invalid path/)
+ expect { check_path_traversal!('foo\\..') }.to raise_error(/Invalid path/)
+ end
+
+ it 'does nothing for a safe string' do
+ expect(check_path_traversal!('./foo')).to eq('./foo')
+ expect(check_path_traversal!('.test/foo')).to eq('.test/foo')
+ expect(check_path_traversal!('..test/foo')).to eq('..test/foo')
+ expect(check_path_traversal!('dir/..foo.rb')).to eq('dir/..foo.rb')
+ expect(check_path_traversal!('dir/.foo.rb')).to eq('dir/.foo.rb')
+ end
+
+ it 'logs potential path traversal attempts' do
+ expect(Gitlab::AppLogger).to receive(:warn)
+ .with(message: "Potential path traversal attempt detected", path: "..")
+ expect { check_path_traversal!('..') }.to raise_error(/Invalid path/)
+ end
+
+ it 'logs does nothing for a safe string' do
+ expect(Gitlab::AppLogger).not_to receive(:warn)
+ .with(message: "Potential path traversal attempt detected", path: "dir/.foo.rb")
+ expect(check_path_traversal!('dir/.foo.rb')).to eq('dir/.foo.rb')
+ end
+
+ it 'does nothing for nil' do
+ expect(check_path_traversal!(nil)).to be_nil
+ end
+
+ it 'does nothing for safe HashedPath' do
+ expect(check_path_traversal!(Gitlab::HashedPath.new('tmp', root_hash: 1)))
+ .to eq '6b/86/6b86b273ff34fce19d6b804eff5a3f5747ada4eaa22f1d49c01e52ddb7875b4b/tmp'
+ end
+
+ it 'raises for unsafe HashedPath' do
+ expect { check_path_traversal!(Gitlab::HashedPath.new('tmp', '..', 'etc', 'passwd', root_hash: 1)) }
+ .to raise_error(/Invalid path/)
+ end
+
+ it 'raises for other non-strings' do
+ expect { check_path_traversal!(%w[/tmp /tmp/../etc/passwd]) }.to raise_error(/Invalid path/)
+ end
+ end
+
+ describe '.check_allowed_absolute_path!' do
+ let(:allowed_paths) { ['/home/foo'] }
+
+ it 'raises an exception if an absolute path is not allowed' do
+ expect { check_allowed_absolute_path!('/etc/passwd', allowed_paths) }.to raise_error(StandardError)
+ end
+
+ it 'does nothing for an allowed absolute path' do
+ expect(check_allowed_absolute_path!('/home/foo', allowed_paths)).to be_nil
+ end
+ end
+
+ describe '.check_allowed_absolute_path_and_path_traversal!' do
+ let(:allowed_paths) { %w[/home/foo ./foo .test/foo ..test/foo dir/..foo.rb dir/.foo.rb] }
+
+ it 'detects path traversal in string without any separators' do
+ expect { check_allowed_absolute_path_and_path_traversal!('.', allowed_paths) }
+ .to raise_error(/Invalid path/)
+ expect { check_allowed_absolute_path_and_path_traversal!('..', allowed_paths) }
+ .to raise_error(/Invalid path/)
+ end
+
+ it 'detects path traversal at the start of the string' do
+ expect { check_allowed_absolute_path_and_path_traversal!('../foo', allowed_paths) }
+ .to raise_error(/Invalid path/)
+ expect { check_allowed_absolute_path_and_path_traversal!('..\\foo', allowed_paths) }
+ .to raise_error(/Invalid path/)
+ end
+
+ it 'detects path traversal at the start of the string, even to just the subdirectory' do
+ expect { check_allowed_absolute_path_and_path_traversal!('../', allowed_paths) }
+ .to raise_error(/Invalid path/)
+ expect { check_allowed_absolute_path_and_path_traversal!('..\\', allowed_paths) }
+ .to raise_error(/Invalid path/)
+ expect { check_allowed_absolute_path_and_path_traversal!('/../', allowed_paths) }
+ .to raise_error(/Invalid path/)
+ expect { check_allowed_absolute_path_and_path_traversal!('\\..\\', allowed_paths) }
+ .to raise_error(/Invalid path/)
+ end
+
+ it 'detects path traversal in the middle of the string' do
+ expect { check_allowed_absolute_path_and_path_traversal!('foo/../../bar', allowed_paths) }
+ .to raise_error(/Invalid path/)
+ expect { check_allowed_absolute_path_and_path_traversal!('foo\\..\\..\\bar', allowed_paths) }
+ .to raise_error(/Invalid path/)
+ expect { check_allowed_absolute_path_and_path_traversal!('foo/..\\bar', allowed_paths) }
+ .to raise_error(/Invalid path/)
+ expect { check_allowed_absolute_path_and_path_traversal!('foo\\../bar', allowed_paths) }
+ .to raise_error(/Invalid path/)
+ expect { check_allowed_absolute_path_and_path_traversal!('foo/..\\..\\..\\..\\../bar', allowed_paths) }
+ .to raise_error(/Invalid path/)
+ end
+
+ it 'detects path traversal at the end of the string when slash-terminates' do
+ expect { check_allowed_absolute_path_and_path_traversal!('foo/../', allowed_paths) }
+ .to raise_error(/Invalid path/)
+ expect { check_allowed_absolute_path_and_path_traversal!('foo\\..\\', allowed_paths) }
+ .to raise_error(/Invalid path/)
+ end
+
+ it 'detects path traversal at the end of the string' do
+ expect { check_allowed_absolute_path_and_path_traversal!('foo/..', allowed_paths) }
+ .to raise_error(/Invalid path/)
+ expect { check_allowed_absolute_path_and_path_traversal!('foo\\..', allowed_paths) }
+ .to raise_error(/Invalid path/)
+ end
+
+ it 'does not return errors for a safe string' do
+ expect(check_allowed_absolute_path_and_path_traversal!('./foo', allowed_paths)).to be_nil
+ expect(check_allowed_absolute_path_and_path_traversal!('.test/foo', allowed_paths)).to be_nil
+ expect(check_allowed_absolute_path_and_path_traversal!('..test/foo', allowed_paths)).to be_nil
+ expect(check_allowed_absolute_path_and_path_traversal!('dir/..foo.rb', allowed_paths)).to be_nil
+ expect(check_allowed_absolute_path_and_path_traversal!('dir/.foo.rb', allowed_paths)).to be_nil
+ end
+
+ it 'raises error for a non-string' do
+ expect { check_allowed_absolute_path_and_path_traversal!(nil, allowed_paths) }.to raise_error(StandardError)
+ end
+
+ it 'raises an exception if an absolute path is not allowed' do
+ expect { check_allowed_absolute_path!('/etc/passwd', allowed_paths) }.to raise_error(StandardError)
+ end
+
+ it 'does nothing for an allowed absolute path' do
+ expect(check_allowed_absolute_path!('/home/foo', allowed_paths)).to be_nil
+ end
+ end
+end
diff --git a/spec/lib/gitlab/utils_spec.rb b/spec/lib/gitlab/utils_spec.rb
index 102d608072bcce7b519b9a90c317da0b9ec1baba..e6ee5d1bea6817112cad76790c55abf7aeddf4c6 100644
--- a/spec/lib/gitlab/utils_spec.rb
+++ b/spec/lib/gitlab/utils_spec.rb
@@ -7,138 +7,8 @@
delegate :to_boolean, :boolean_to_yes_no, :slugify, :which,
:ensure_array_from_string, :to_exclusive_sentence, :bytes_to_megabytes,
- :append_path, :remove_leading_slashes, :check_path_traversal!, :allowlisted?, :check_allowed_absolute_path!,
- :decode_path, :ms_to_round_sec, :check_allowed_absolute_path_and_path_traversal!, to: :described_class
-
- describe '.check_path_traversal!' do
- it 'detects path traversal in string without any separators' do
- expect { check_path_traversal!('.') }.to raise_error(/Invalid path/)
- expect { check_path_traversal!('..') }.to raise_error(/Invalid path/)
- end
-
- it 'detects path traversal at the start of the string' do
- expect { check_path_traversal!('../foo') }.to raise_error(/Invalid path/)
- expect { check_path_traversal!('..\\foo') }.to raise_error(/Invalid path/)
- end
-
- it 'detects path traversal at the start of the string, even to just the subdirectory' do
- expect { check_path_traversal!('../') }.to raise_error(/Invalid path/)
- expect { check_path_traversal!('..\\') }.to raise_error(/Invalid path/)
- expect { check_path_traversal!('/../') }.to raise_error(/Invalid path/)
- expect { check_path_traversal!('\\..\\') }.to raise_error(/Invalid path/)
- end
-
- it 'detects path traversal in the middle of the string' do
- expect { check_path_traversal!('foo/../../bar') }.to raise_error(/Invalid path/)
- expect { check_path_traversal!('foo\\..\\..\\bar') }.to raise_error(/Invalid path/)
- expect { check_path_traversal!('foo/..\\bar') }.to raise_error(/Invalid path/)
- expect { check_path_traversal!('foo\\../bar') }.to raise_error(/Invalid path/)
- expect { check_path_traversal!('foo/..\\..\\..\\..\\../bar') }.to raise_error(/Invalid path/)
- end
-
- it 'detects path traversal at the end of the string when slash-terminates' do
- expect { check_path_traversal!('foo/../') }.to raise_error(/Invalid path/)
- expect { check_path_traversal!('foo\\..\\') }.to raise_error(/Invalid path/)
- end
-
- it 'detects path traversal at the end of the string' do
- expect { check_path_traversal!('foo/..') }.to raise_error(/Invalid path/)
- expect { check_path_traversal!('foo\\..') }.to raise_error(/Invalid path/)
- end
-
- it 'does nothing for a safe string' do
- expect(check_path_traversal!('./foo')).to eq('./foo')
- expect(check_path_traversal!('.test/foo')).to eq('.test/foo')
- expect(check_path_traversal!('..test/foo')).to eq('..test/foo')
- expect(check_path_traversal!('dir/..foo.rb')).to eq('dir/..foo.rb')
- expect(check_path_traversal!('dir/.foo.rb')).to eq('dir/.foo.rb')
- end
-
- it 'logs potential path traversal attempts' do
- expect(Gitlab::AppLogger).to receive(:warn).with(message: "Potential path traversal attempt detected", path: "..")
- expect { check_path_traversal!('..') }.to raise_error(/Invalid path/)
- end
-
- it 'logs does nothing for a safe string' do
- expect(Gitlab::AppLogger).not_to receive(:warn).with(message: "Potential path traversal attempt detected", path: "dir/.foo.rb")
- expect(check_path_traversal!('dir/.foo.rb')).to eq('dir/.foo.rb')
- end
-
- it 'does nothing for nil' do
- expect(check_path_traversal!(nil)).to be_nil
- end
-
- it 'does nothing for safe HashedPath' do
- expect(check_path_traversal!(Gitlab::HashedPath.new('tmp', root_hash: 1))).to eq '6b/86/6b86b273ff34fce19d6b804eff5a3f5747ada4eaa22f1d49c01e52ddb7875b4b/tmp'
- end
-
- it 'raises for unsafe HashedPath' do
- expect { check_path_traversal!(Gitlab::HashedPath.new('tmp', '..', 'etc', 'passwd', root_hash: 1)) }.to raise_error(/Invalid path/)
- end
-
- it 'raises for other non-strings' do
- expect { check_path_traversal!(%w[/tmp /tmp/../etc/passwd]) }.to raise_error(/Invalid path/)
- end
- end
-
- describe '.check_allowed_absolute_path_and_path_traversal!' do
- let(:allowed_paths) { %w[/home/foo ./foo .test/foo ..test/foo dir/..foo.rb dir/.foo.rb] }
-
- it 'detects path traversal in string without any separators' do
- expect { check_allowed_absolute_path_and_path_traversal!('.', allowed_paths) }.to raise_error(/Invalid path/)
- expect { check_allowed_absolute_path_and_path_traversal!('..', allowed_paths) }.to raise_error(/Invalid path/)
- end
-
- it 'detects path traversal at the start of the string' do
- expect { check_allowed_absolute_path_and_path_traversal!('../foo', allowed_paths) }.to raise_error(/Invalid path/)
- expect { check_allowed_absolute_path_and_path_traversal!('..\\foo', allowed_paths) }.to raise_error(/Invalid path/)
- end
-
- it 'detects path traversal at the start of the string, even to just the subdirectory' do
- expect { check_allowed_absolute_path_and_path_traversal!('../', allowed_paths) }.to raise_error(/Invalid path/)
- expect { check_allowed_absolute_path_and_path_traversal!('..\\', allowed_paths) }.to raise_error(/Invalid path/)
- expect { check_allowed_absolute_path_and_path_traversal!('/../', allowed_paths) }.to raise_error(/Invalid path/)
- expect { check_allowed_absolute_path_and_path_traversal!('\\..\\', allowed_paths) }.to raise_error(/Invalid path/)
- end
-
- it 'detects path traversal in the middle of the string' do
- expect { check_allowed_absolute_path_and_path_traversal!('foo/../../bar', allowed_paths) }.to raise_error(/Invalid path/)
- expect { check_allowed_absolute_path_and_path_traversal!('foo\\..\\..\\bar', allowed_paths) }.to raise_error(/Invalid path/)
- expect { check_allowed_absolute_path_and_path_traversal!('foo/..\\bar', allowed_paths) }.to raise_error(/Invalid path/)
- expect { check_allowed_absolute_path_and_path_traversal!('foo\\../bar', allowed_paths) }.to raise_error(/Invalid path/)
- expect { check_allowed_absolute_path_and_path_traversal!('foo/..\\..\\..\\..\\../bar', allowed_paths) }.to raise_error(/Invalid path/)
- end
-
- it 'detects path traversal at the end of the string when slash-terminates' do
- expect { check_allowed_absolute_path_and_path_traversal!('foo/../', allowed_paths) }.to raise_error(/Invalid path/)
- expect { check_allowed_absolute_path_and_path_traversal!('foo\\..\\', allowed_paths) }.to raise_error(/Invalid path/)
- end
-
- it 'detects path traversal at the end of the string' do
- expect { check_allowed_absolute_path_and_path_traversal!('foo/..', allowed_paths) }.to raise_error(/Invalid path/)
- expect { check_allowed_absolute_path_and_path_traversal!('foo\\..', allowed_paths) }.to raise_error(/Invalid path/)
- end
-
- it 'does not return errors for a safe string' do
- expect(check_allowed_absolute_path_and_path_traversal!('./foo', allowed_paths)).to be_nil
- expect(check_allowed_absolute_path_and_path_traversal!('.test/foo', allowed_paths)).to be_nil
- expect(check_allowed_absolute_path_and_path_traversal!('..test/foo', allowed_paths)).to be_nil
- expect(check_allowed_absolute_path_and_path_traversal!('dir/..foo.rb', allowed_paths)).to be_nil
- expect(check_allowed_absolute_path_and_path_traversal!('dir/.foo.rb', allowed_paths)).to be_nil
- end
-
- it 'raises error for a non-string' do
- expect { check_allowed_absolute_path_and_path_traversal!(nil, allowed_paths) }.to raise_error(StandardError)
- end
-
- it 'raises an exception if an absolute path is not allowed' do
- expect { check_allowed_absolute_path!('/etc/passwd', allowed_paths) }.to raise_error(StandardError)
- end
-
- it 'does nothing for an allowed absolute path' do
- expect(check_allowed_absolute_path!('/home/foo', allowed_paths)).to be_nil
- end
- end
+ :append_path, :remove_leading_slashes, :allowlisted?,
+ :decode_path, :ms_to_round_sec, to: :described_class
describe '.allowlisted?' do
let(:allowed_paths) { ['/home/foo', '/foo/bar', '/etc/passwd'] }
@@ -152,18 +22,6 @@
end
end
- describe '.check_allowed_absolute_path!' do
- let(:allowed_paths) { ['/home/foo'] }
-
- it 'raises an exception if an absolute path is not allowed' do
- expect { check_allowed_absolute_path!('/etc/passwd', allowed_paths) }.to raise_error(StandardError)
- end
-
- it 'does nothing for an allowed absolute path' do
- expect(check_allowed_absolute_path!('/home/foo', allowed_paths)).to be_nil
- end
- end
-
describe '.decode_path' do
it 'returns path unencoded for singled-encoded paths' do
expect(decode_path('%2Fhome%2Fbar%3Fasd%3Dqwe')).to eq('/home/bar?asd=qwe')
diff --git a/spec/models/merge_request_diff_spec.rb b/spec/models/merge_request_diff_spec.rb
index 1ecc43566721f663cae8f611ef9cff86c84c41fd..e2c87b0d85cdcfe3133e81a718b96d91b770f8b9 100644
--- a/spec/models/merge_request_diff_spec.rb
+++ b/spec/models/merge_request_diff_spec.rb
@@ -1274,7 +1274,7 @@
it 'raises' do
allow(diff).to receive(:external_diff_cache_dir).and_return(File.join(cache_dir, '..'))
- expect { diff.remove_cached_external_diff }.to raise_error(Gitlab::Utils::PathTraversalAttackError, 'Invalid path')
+ expect { diff.remove_cached_external_diff }.to raise_error(Gitlab::PathTraversal::PathTraversalAttackError, 'Invalid path')
end
end
diff --git a/spec/services/bulk_imports/archive_extraction_service_spec.rb b/spec/services/bulk_imports/archive_extraction_service_spec.rb
index 40f8d8718ae410b41fcbff03ee4d1abf08a1b958..5593218c25905ffd159c0f209e8e09aef75c0c30 100644
--- a/spec/services/bulk_imports/archive_extraction_service_spec.rb
+++ b/spec/services/bulk_imports/archive_extraction_service_spec.rb
@@ -53,7 +53,7 @@
context 'when filepath is being traversed' do
it 'raises an error' do
expect { described_class.new(tmpdir: File.join(Dir.mktmpdir, 'test', '..'), filename: 'name').execute }
- .to raise_error(Gitlab::Utils::PathTraversalAttackError, 'Invalid path')
+ .to raise_error(Gitlab::PathTraversal::PathTraversalAttackError, 'Invalid path')
end
end
end
diff --git a/spec/services/bulk_imports/file_decompression_service_spec.rb b/spec/services/bulk_imports/file_decompression_service_spec.rb
index 9b8320aeac5320c692b5619e657d8148cbc8f7ce..9d80ab3cd8fbd1fd87ee5f1a733adf74b320d04e 100644
--- a/spec/services/bulk_imports/file_decompression_service_spec.rb
+++ b/spec/services/bulk_imports/file_decompression_service_spec.rb
@@ -66,7 +66,7 @@
subject { described_class.new(tmpdir: File.join(Dir.mktmpdir, 'test', '..'), filename: 'filename') }
it 'raises an error' do
- expect { subject.execute }.to raise_error(Gitlab::Utils::PathTraversalAttackError, 'Invalid path')
+ expect { subject.execute }.to raise_error(Gitlab::PathTraversal::PathTraversalAttackError, 'Invalid path')
end
end
diff --git a/spec/services/bulk_imports/file_download_service_spec.rb b/spec/services/bulk_imports/file_download_service_spec.rb
index 7c64d6efc6525156144edebc3cdddaedf0113172..4aa9cf3a73daaa1411550cf85cf0775a0a26858d 100644
--- a/spec/services/bulk_imports/file_download_service_spec.rb
+++ b/spec/services/bulk_imports/file_download_service_spec.rb
@@ -281,7 +281,7 @@
it 'raises an error' do
expect { subject.execute }.to raise_error(
- Gitlab::Utils::PathTraversalAttackError,
+ Gitlab::PathTraversal::PathTraversalAttackError,
'Invalid path'
)
end
diff --git a/spec/services/projects/readme_renderer_service_spec.rb b/spec/services/projects/readme_renderer_service_spec.rb
index 842d75e82eea989c70d59fabc73fa85219b51e93..2360535b7386aed769ce0e9e05fe09130cc768bc 100644
--- a/spec/services/projects/readme_renderer_service_spec.rb
+++ b/spec/services/projects/readme_renderer_service_spec.rb
@@ -52,7 +52,7 @@
context 'with path traversal in mind' do
where(:template_name, :exception, :expected_path) do
- '../path/traversal/bad' | [Gitlab::Utils::PathTraversalAttackError, 'Invalid path'] | nil
+ '../path/traversal/bad' | [Gitlab::PathTraversal::PathTraversalAttackError, 'Invalid path'] | nil
'/bad/template' | [StandardError, 'path /bad/template.md.tt is not allowed'] | nil
'good/template' | nil | 'good/template.md.tt'
end
diff --git a/spec/support/shared_examples/uploaders/gitlab_uploader_shared_examples.rb b/spec/support/shared_examples/uploaders/gitlab_uploader_shared_examples.rb
index 7126d3ace96db64a992f795bed0abc67233d1de0..a7e5892d439290a0ab44feb329befc6291a90a5c 100644
--- a/spec/support/shared_examples/uploaders/gitlab_uploader_shared_examples.rb
+++ b/spec/support/shared_examples/uploaders/gitlab_uploader_shared_examples.rb
@@ -63,8 +63,8 @@
end
it "throws an exception" do
- expect { subject.cache!(fixture_file_upload(fixture)) }.to raise_error(Gitlab::Utils::PathTraversalAttackError)
- expect { subject.store!(fixture_file_upload(fixture)) }.to raise_error(Gitlab::Utils::PathTraversalAttackError)
+ expect { subject.cache!(fixture_file_upload(fixture)) }.to raise_error(Gitlab::PathTraversal::PathTraversalAttackError)
+ expect { subject.store!(fixture_file_upload(fixture)) }.to raise_error(Gitlab::PathTraversal::PathTraversalAttackError)
end
end
end