Add latest changes from gitlab-org/security/gitlab@12-10-stable-ee

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 41d54ee0a92..18c96836253 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?
         }
       end
@@ -47,6 +47,10 @@ module Gitlab
       def file_exists?
         commit_id.present? && repository.blob_at(commit_id, file_path).present?
       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 8f61476722d..8fd4c844375 100644
--- a/spec/lib/gitlab/static_site_editor/config_spec.rb
+++ b/spec/lib/gitlab/static_site_editor/config_spec.rb
@@ -57,5 +57,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])