Merge branch 'bvl-markup-pipeline' into 'security'

Render asciidoc & other markup using banzai in a pipeline

See merge request !2088

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

@@ -30,15 +30,14 @@ module Gitlab
       )
       asciidoc_opts[:attributes].unshift(*DEFAULT_ADOC_ATTRS)
 
+      context[:pipeline] = :markup
+
       plantuml_setup
 
       html = ::Asciidoctor.convert(input, asciidoc_opts)
-
+      html = Banzai.render(html, context)
       html = Banzai.post_process(html, context)
 
-      filter = Banzai::Filter::SanitizationFilter.new(html)
-      html = filter.call.to_s
-
       html.html_safe
     end
 

lib/gitlab/other_markup.rb

@@ -14,12 +14,11 @@ module Gitlab
     def self.render(file_name, input, context)
       html = GitHub::Markup.render(file_name, input).
         force_encoding(input.encoding)
+      context[:pipeline] = :markup
 
+      html = Banzai.render(html, context)
       html = Banzai.post_process(html, context)
 
-      filter = Banzai::Filter::SanitizationFilter.new(html)
-      html = filter.call.to_s
-
       html.html_safe
     end
   end

spec/lib/gitlab/asciidoc_spec.rb

@@ -50,7 +50,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>',
@@ -60,10 +60,18 @@ module Gitlab
 
         links.each do |name, data|
           it "does not convert dangerous #{name} into HTML" do
-            expect(render(data[:input], context)).to eql 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 noopener"')
+        end
+      end
     end
 
     def render(*args)

spec/lib/gitlab/other_markup.rb

-require 'spec_helper'
-
-describe Gitlab::OtherMarkup, lib: true do
-  context "XSS Checks" do
-    links = {
-      'links' => {
-        file: 'file.rdoc',
-        input: 'XSS[JaVaScriPt:alert(1)]',
-        output: '<p><a>XSS</a></p>'
-      }
-    }
-    links.each do |name, data|
-      it "does not convert dangerous #{name} into HTML" do
-        expect(render(data[:file], data[:input], context)).to eql data[:output]
-      end
-    end
-  end
-
-  def render(*args)
-    described_class.render(*args)
-  end
-end

spec/lib/gitlab/other_markup_spec.rb

+require 'spec_helper'
+
+describe Gitlab::OtherMarkup, lib: true do
+  context "XSS Checks" do
+    it "does not convert dangerous #{name} into HTML" do
+      context = {}
+      filename = 'file.rdoc'
+      input = 'XSS[JaVaScriPt:alert(1)]'
+      output = '<p><a>XSS</a></p>'
+
+      expect(render(filename, input, context)).to eql output
+    end
+  end
+
+  context 'external links' do
+    it 'adds the `rel` attribute to the link' do
+      context = {}
+      output = render('file.rdoc', '{Google}[https://google.com]', context)
+
+      expect(output).to include('rel="nofollow noreferrer noopener"')
+    end
+  end
+
+  def render(*args)
+    described_class.render(*args).strip
+  end
+end