Commit 0aa80409 authored by Kerri Miller's avatar Kerri Miller

Merge branch '255184' into 'master'

Logs when potential path traversal attempt detected

See merge request gitlab-org/gitlab!81241
parents 4752b221 abf83429
......@@ -5,6 +5,10 @@ module Gitlab
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
......@@ -16,6 +20,7 @@ module Gitlab
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
......
......@@ -53,6 +53,16 @@ RSpec.describe Gitlab::Utils do
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 a non-string' do
expect(check_path_traversal!(nil)).to be_nil
end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment