Merge branch 'rs-sanitize-unicode-in-protocol' into 'security-10-0'

[10.0] Prevent a persistent XSS in user-provided markup

See merge request gitlab/gitlabhq!2199

3 files changed, 22 insertions, 2 deletions

diff --git a/changelogs/unreleased/rs-sanitize-unicode-in-protocol.yml b/changelogs/unreleased/rs-sanitize-unicode-in-protocol.yml
new file mode 100644
index 00000000000..093c99943e2
--- /dev/null
+++ b/changelogs/unreleased/rs-sanitize-unicode-in-protocol.yml
@@ -0,0 +1,5 @@
+---
+title: Prevent a persistent XSS in user-provided markup
+merge_request:
+author:
+type: security
diff --git a/lib/banzai/filter/sanitization_filter.rb b/lib/banzai/filter/sanitization_filter.rb
index 88b17e12576..6735a346598 100644
--- a/lib/banzai/filter/sanitization_filter.rb
+++ b/lib/banzai/filter/sanitization_filter.rb
@@ -74,9 +74,19 @@ module Banzai
 
             begin
               uri = Addressable::URI.parse(node['href'])
-              uri.scheme = uri.scheme.strip.downcase if uri.scheme
 
-              node.remove_attribute('href') if UNSAFE_PROTOCOLS.include?(uri.scheme)
+              return unless uri.scheme
+
+              # Remove all invalid scheme characters before checking against the
+              # list of unsafe protocols.
+              #
+              # See https://tools.ietf.org/html/rfc3986#section-3.1
+              scheme = uri.scheme
+                .strip
+                .downcase
+                .gsub(/[^A-Za-z0-9\+\.\-]+/, '')
+
+              node.remove_attribute('href') if UNSAFE_PROTOCOLS.include?(scheme)
             rescue Addressable::URI::InvalidURIError
               node.remove_attribute('href')
             end
diff --git a/spec/lib/banzai/filter/sanitization_filter_spec.rb b/spec/lib/banzai/filter/sanitization_filter_spec.rb
index 5f41e28fece..17a620ef603 100644
--- a/spec/lib/banzai/filter/sanitization_filter_spec.rb
+++ b/spec/lib/banzai/filter/sanitization_filter_spec.rb
@@ -217,6 +217,11 @@ describe Banzai::Filter::SanitizationFilter do
         output: '<img>'
       },
 
+      'protocol-based JS injection: Unicode' => {
+        input: %Q(<a href="\u0001java\u0003script:alert('XSS')">foo</a>),
+        output: '<a>foo</a>'
+      },
+
       'protocol-based JS injection: spaces and entities' => {
         input:  '<a href=" &#14;  javascript:alert(\'XSS\');">foo</a>',
         output: '<a href="">foo</a>'