Reject slug+uri concat if slug is deemed unsafe

First reported:
  https://gitlab.com/gitlab-org/gitlab-ce/issues/60143

When the page slug is "javascript:" and we attempt to link to a relative
path (using `.` or `..`) the code will concatenate the slug and the uri.
This MR adds a guard to that concat step that will return `nil` if the
incoming slug matches against any of the "unsafe" slug regexes;
currently this is only for the slug "javascript:" but can be extended if
needed. Manually tested against a non-exhaustive list from OWASP of
common javascript XSS exploits that have to to with mangling the
"javascript:" method, and all are caught by this change or by existing
code that ingests the user-specified slug.

3 files changed, 55 insertions, 0 deletions

diff --git a/changelogs/unreleased/security-60143-address-xss-issue-in-wiki-links.yml b/changelogs/unreleased/security-60143-address-xss-issue-in-wiki-links.yml
new file mode 100644
index 00000000000..5b79258af54
--- /dev/null
+++ b/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
diff --git a/lib/banzai/filter/wiki_link_filter/rewriter.rb b/lib/banzai/filter/wiki_link_filter/rewriter.rb
index f4cc8beeb52..77b5053f38c 100644
--- a/lib/banzai/filter/wiki_link_filter/rewriter.rb
+++ b/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
diff --git a/spec/lib/banzai/filter/wiki_link_filter_spec.rb b/spec/lib/banzai/filter/wiki_link_filter_spec.rb
index b9059b85fdc..cce1cd0b284 100644
--- a/spec/lib/banzai/filter/wiki_link_filter_spec.rb
+++ b/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&#58;",
+        "javascript&#0058;",
+        "javascript&#x3A;",
+        "javascript&#x003A;",
+        "java\0script:",
+        " &#14;  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