Merge branch 'security-60143-address-xss-issue-master' into 'master'

Reject slug+uri concat if slug is deemed unsafe

See merge request gitlab/gitlabhq!3108

changelogs/unreleased/security-60143-address-xss-issue-in-wiki-links.yml

@@ -0,0 +1,5 @@
+---
+title: Filter relative links in wiki for XSS
+merge_request:
+author:
+type: security

lib/banzai/filter/wiki_link_filter/rewriter.rb

@@ -4,6 +4,8 @@ module Banzai
   module Filter
     class WikiLinkFilter < HTML::Pipeline::Filter
       class Rewriter
+        UNSAFE_SLUG_REGEXES = [/\Ajavascript:/i].freeze
+
         def initialize(link_string, wiki:, slug:)
           @uri = Addressable::URI.parse(link_string)
           @wiki_base_path = wiki && wiki.wiki_base_path
@@ -35,6 +37,8 @@ module Banzai
 
         # Of the form `./link`, `../link`, or similar
         def apply_hierarchical_link_rules!
+          return if slug_considered_unsafe?
+
           @uri = Addressable::URI.join(@slug, @uri) if @uri.to_s[0] == '.'
         end
 
@@ -54,6 +58,10 @@ module Banzai
         def repository_upload?
           @uri.relative? && @uri.path.starts_with?(Wikis::CreateAttachmentService::ATTACHMENT_PATH)
         end
+
+        def slug_considered_unsafe?
+          UNSAFE_SLUG_REGEXES.any? { |r| r.match?(@slug) }
+        end
       end
     end
   end

spec/lib/banzai/filter/wiki_link_filter_spec.rb

@@ -70,5 +70,47 @@ describe Banzai::Filter::WikiLinkFilter do
         expect(filtered_link.attribute('href').value).to eq(invalid_link)
       end
     end
+
+    context "when the slug is deemed unsafe or invalid" do
+      let(:link) { "alert(1);" }
+
+      invalid_slugs = [
+        "javascript:",
+        "JaVaScRiPt:",
+        "\u0001java\u0003script:",
+        "javascript    :",
+        "javascript:    ",
+        "javascript    :   ",
+        ":javascript:",
+        "javascript:",
+        "javascript:",
+        "javascript:",
+        "javascript:",
+        "java\0script:",
+        "   javascript:"
+        ]
+
+      invalid_slugs.each do |slug|
+        context "with the slug #{slug}" do
+          it "doesn't rewrite a (.) relative link" do
+            filtered_link = filter(
+              "<a href='.#{link}'>Link</a>",
+              project_wiki: wiki,
+              page_slug: slug).children[0]
+
+            expect(filtered_link.attribute('href').value).not_to include(slug)
+          end
+
+          it "doesn't rewrite a (..) relative link" do
+            filtered_link = filter(
+              "<a href='..#{link}'>Link</a>",
+              project_wiki: wiki,
+              page_slug: slug).children[0]
+
+            expect(filtered_link.attribute('href').value).not_to include(slug)
+          end
+        end
+      end
+    end
   end
 end