Commit f5a5ffae authored by Vasilii Iakliushin's avatar Vasilii Iakliushin

Add validation for return URL

Issue: https://gitlab.com/gitlab-org/gitlab/-/issues/216528

Add a validation for the URL scheme. Only `http` and `https` scheme
are allowed.
parent 3ed37596
---
title: Add an extra validation to Static Site Editor payload
merge_request:
author:
type: security
......@@ -21,7 +21,7 @@ module Gitlab
project_id: project.id,
project: project.path,
namespace: project.namespace.path,
return_url: return_url,
return_url: sanitize_url(return_url),
is_supported_content: supported_content?.to_s,
base_url: Gitlab::Routing.url_helpers.project_show_sse_path(project, full_path)
}
......@@ -52,6 +52,10 @@ module Gitlab
def full_path
"#{ref}/#{file_path}"
end
def sanitize_url(url)
url if Gitlab::UrlSanitizer.valid_web?(url)
end
end
end
end
......@@ -3,6 +3,7 @@
module Gitlab
class UrlSanitizer
ALLOWED_SCHEMES = %w[http https ssh git].freeze
ALLOWED_WEB_SCHEMES = %w[http https].freeze
def self.sanitize(content)
regexp = URI::DEFAULT_PARSER.make_regexp(ALLOWED_SCHEMES)
......@@ -12,17 +13,21 @@ module Gitlab
content.gsub(regexp, '')
end
def self.valid?(url)
def self.valid?(url, allowed_schemes: ALLOWED_SCHEMES)
return false unless url.present?
return false unless url.is_a?(String)
uri = Addressable::URI.parse(url.strip)
ALLOWED_SCHEMES.include?(uri.scheme)
allowed_schemes.include?(uri.scheme)
rescue Addressable::URI::InvalidURIError
false
end
def self.valid_web?(url)
valid?(url, allowed_schemes: ALLOWED_WEB_SCHEMES)
end
def initialize(url, credentials: nil)
%i[user password].each do |symbol|
credentials[symbol] = credentials[symbol].presence if credentials&.key?(symbol)
......
......@@ -65,5 +65,23 @@ describe Gitlab::StaticSiteEditor::Config do
it { is_expected.to include(is_supported_content: 'false') }
end
context 'when return_url is not a valid URL' do
let(:return_url) { 'example.com' }
it { is_expected.to include(return_url: nil) }
end
context 'when return_url has a javascript scheme' do
let(:return_url) { 'javascript:alert(document.domain)' }
it { is_expected.to include(return_url: nil) }
end
context 'when return_url is missing' do
let(:return_url) { nil }
it { is_expected.to include(return_url: nil) }
end
end
end
......@@ -60,6 +60,30 @@ describe Gitlab::UrlSanitizer do
end
end
describe '.valid_web?' do
where(:value, :url) do
false | nil
false | ''
false | '123://invalid:url'
false | 'valid@project:url.git'
false | 'valid:pass@project:url.git'
false | %w(test array)
false | 'ssh://example.com'
false | 'ssh://:@example.com'
false | 'ssh://foo@example.com'
false | 'ssh://foo:bar@example.com'
false | 'ssh://foo:bar@example.com/group/group/project.git'
false | 'git://example.com/group/group/project.git'
false | 'git://foo:bar@example.com/group/group/project.git'
true | 'http://foo:bar@example.com/group/group/project.git'
true | 'https://foo:bar@example.com/group/group/project.git'
end
with_them do
it { expect(described_class.valid_web?(url)).to eq(value) }
end
end
describe '#sanitized_url' do
context 'credentials in hash' do
where(username: ['foo', '', nil], password: ['bar', '', nil])
......
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