Merge branch 'bvl-security-9-1-markup-pipeline'

(security-9-1) Render asciidoc & other markup using banzai in a pipeline

See merge request !2098

app/helpers/markup_helper.rb

@@ -116,13 +116,13 @@ module MarkupHelper
     if gitlab_markdown?(file_name)
       markdown_unsafe(text, context)
     elsif asciidoc?(file_name)
-      asciidoc_unsafe(text)
+      asciidoc_unsafe(text, context)
     elsif plain?(file_name)
       content_tag :pre, class: 'plain-readme' do
         text
       end
     else
-      other_markup_unsafe(file_name, text)
+      other_markup_unsafe(file_name, text, context)
     end
   rescue RuntimeError
     simple_format(text)
@@ -217,12 +217,12 @@ module MarkupHelper
     Banzai.render(text, context)
   end
-  def asciidoc_unsafe(text)
-    Gitlab::Asciidoc.render(text)
+  def asciidoc_unsafe(text, context = {})
+    Gitlab::Asciidoc.render(text, context)
   end
-  def other_markup_unsafe(file_name, text)
-    Gitlab::OtherMarkup.render(file_name, text)
+  def other_markup_unsafe(file_name, text, context = {})
+    Gitlab::OtherMarkup.render(file_name, text, context)
   end
   def prepare_for_rendering(html, context = {})

changelogs/unreleased/bvl-markup-pipeline.yml

+---
+title: Make Asciidoc & other markup go through pipeline to prevent XSS
+merge_request:
+author:

lib/banzai/pipeline/markup_pipeline.rb

+module Banzai
+  module Pipeline
+    class MarkupPipeline < BasePipeline
+      def self.filters
+        @filters ||= FilterArray[
+          Filter::SanitizationFilter,
+          Filter::ExternalLinkFilter,
+          Filter::PlantumlFilter
+        ]
+      end
+    end
+  end
+end

lib/gitlab/asciidoc.rb

@@ -15,17 +15,17 @@ module Gitlab
     #
     # input         - the source text in Asciidoc format
     #
-    def self.render(input)
+    def self.render(input, context)
       asciidoc_opts = { safe: :secure,
                         backend: :gitlab_html5,
                         attributes: DEFAULT_ADOC_ATTRS }
+      context[:pipeline] = :markup
+
       plantuml_setup
       html = ::Asciidoctor.convert(input, asciidoc_opts)
-
-      filter = Banzai::Filter::SanitizationFilter.new(html)
-      html = filter.call.to_s
+      html = Banzai.render(html, context)
       html.html_safe
     end

lib/gitlab/other_markup.rb

@@ -5,12 +5,12 @@ module Gitlab
     #
     # input         - the source text in a markup format
     #
-    def self.render(file_name, input)
+    def self.render(file_name, input, context)
       html = GitHub::Markup.render(file_name, input).
         force_encoding(input.encoding)
-      filter = Banzai::Filter::SanitizationFilter.new(html)
-      html = filter.call.to_s
+      context[:pipeline] = :markup
+      html = Banzai.render(html, context)
       html.html_safe
     end

spec/lib/gitlab/asciidoc_spec.rb

@@ -22,7 +22,7 @@ module Gitlab
         expect(Asciidoctor).to receive(:convert)
           .with(input, expected_asciidoc_opts).and_return(html)
-        expect(render(input)).to eq(html)
+        expect(render(input, context)).to eq(html)
       end
       context "XSS" do
@@ -33,7 +33,7 @@ module Gitlab
           },
           'images' => {
             input: 'image:https://localhost.com/image.png[Alt text" onerror="alert(7)]',
-            output: "<div>\n<p><span><img src=\"https://localhost.com/image.png\" alt=\"Alt text\"></span></p>\n</div>"
+            output: "<img src=\"https://localhost.com/image.png\" alt=\"Alt text\">"
           },
           'pre' => {
             input: '```mypre"><script>alert(3)</script>',
@@ -43,10 +43,18 @@ module Gitlab
         links.each do |name, data|
           it "does not convert dangerous #{name} into HTML" do
-            expect(render(data[:input])).to eq(data[:output])
+            expect(render(data[:input], context)).to include(data[:output])
           end
         end
       end
+
+      context 'external links' do
+        it 'adds the `rel` attribute to the link' do
+          output = render('link:https://google.com[Google]', context)
+
+          expect(output).to include('rel="nofollow noreferrer"')
+        end
+      end
     end
     def render(*args)

spec/lib/gitlab/other_markup_spec.rb

@@ -13,7 +13,7 @@ describe Gitlab::OtherMarkup, lib: true do
     }
     links.each do |name, data|
       it "does not convert dangerous #{name} into HTML" do
-        expect(render(data[:file], data[:input])).to eq(data[:output])
+        expect(render(data[:file], data[:input], context)).to eq(data[:output])
       end
     end
   end