diff options
-rw-r--r-- | changelogs/unreleased/216528-confidential-issue.yml | 5 | ||||
-rw-r--r-- | lib/gitlab/static_site_editor/config.rb | 6 | ||||
-rw-r--r-- | lib/gitlab/url_sanitizer.rb | 9 | ||||
-rw-r--r-- | spec/lib/gitlab/static_site_editor/config_spec.rb | 18 | ||||
-rw-r--r-- | spec/lib/gitlab/url_sanitizer_spec.rb | 24 |
5 files changed, 59 insertions, 3 deletions
diff --git a/changelogs/unreleased/216528-confidential-issue.yml b/changelogs/unreleased/216528-confidential-issue.yml new file mode 100644 index 00000000000..8d9d882e64d --- /dev/null +++ b/changelogs/unreleased/216528-confidential-issue.yml @@ -0,0 +1,5 @@ +--- +title: Add an extra validation to Static Site Editor payload +merge_request: +author: +type: security diff --git a/lib/gitlab/static_site_editor/config.rb b/lib/gitlab/static_site_editor/config.rb index c931cdecbeb..65c567ec2a6 100644 --- a/lib/gitlab/static_site_editor/config.rb +++ b/lib/gitlab/static_site_editor/config.rb @@ -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 diff --git a/lib/gitlab/url_sanitizer.rb b/lib/gitlab/url_sanitizer.rb index 215454fe63c..fa40a8b678b 100644 --- a/lib/gitlab/url_sanitizer.rb +++ b/lib/gitlab/url_sanitizer.rb @@ -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) diff --git a/spec/lib/gitlab/static_site_editor/config_spec.rb b/spec/lib/gitlab/static_site_editor/config_spec.rb index a1db567db1a..4cfda83b8f6 100644 --- a/spec/lib/gitlab/static_site_editor/config_spec.rb +++ b/spec/lib/gitlab/static_site_editor/config_spec.rb @@ -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 diff --git a/spec/lib/gitlab/url_sanitizer_spec.rb b/spec/lib/gitlab/url_sanitizer_spec.rb index b39609c594b..caca22eb98b 100644 --- a/spec/lib/gitlab/url_sanitizer_spec.rb +++ b/spec/lib/gitlab/url_sanitizer_spec.rb @@ -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]) |