diff options
author | Robert Speicher <robert@gitlab.com> | 2017-05-02 14:52:19 +0000 |
---|---|---|
committer | Bob Van Landuyt <bob@gitlab.com> | 2017-05-10 16:44:20 +0200 |
commit | 99996b6bc7c13e7e7f871919942907b380d4b58c (patch) | |
tree | c87b232ade284781ae96d2819636c639d584972b | |
parent | 9b03ed0a182c9ad96957085826cc85f8f229569c (diff) | |
download | gitlab-ce-99996b6bc7c13e7e7f871919942907b380d4b58c.tar.gz |
Merge branch 'bvl-security-9-1-markup-pipeline'
(security-9-1) Render asciidoc & other markup using banzai in a pipeline
See merge request !2098
-rw-r--r-- | app/helpers/markup_helper.rb | 12 | ||||
-rw-r--r-- | changelogs/unreleased/bvl-markup-pipeline.yml | 4 | ||||
-rw-r--r-- | lib/banzai/pipeline/markup_pipeline.rb | 13 | ||||
-rw-r--r-- | lib/gitlab/asciidoc.rb | 8 | ||||
-rw-r--r-- | lib/gitlab/other_markup.rb | 6 | ||||
-rw-r--r-- | spec/lib/gitlab/asciidoc_spec.rb | 14 | ||||
-rw-r--r-- | spec/lib/gitlab/other_markup_spec.rb | 2 |
7 files changed, 42 insertions, 17 deletions
diff --git a/app/helpers/markup_helper.rb b/app/helpers/markup_helper.rb index b241a14740b..b636233c426 100644 --- a/app/helpers/markup_helper.rb +++ b/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 = {}) diff --git a/changelogs/unreleased/bvl-markup-pipeline.yml b/changelogs/unreleased/bvl-markup-pipeline.yml new file mode 100644 index 00000000000..d73bad03340 --- /dev/null +++ b/changelogs/unreleased/bvl-markup-pipeline.yml @@ -0,0 +1,4 @@ +--- +title: Make Asciidoc & other markup go through pipeline to prevent XSS +merge_request: +author: diff --git a/lib/banzai/pipeline/markup_pipeline.rb b/lib/banzai/pipeline/markup_pipeline.rb new file mode 100644 index 00000000000..c56d908009f --- /dev/null +++ b/lib/banzai/pipeline/markup_pipeline.rb @@ -0,0 +1,13 @@ +module Banzai + module Pipeline + class MarkupPipeline < BasePipeline + def self.filters + @filters ||= FilterArray[ + Filter::SanitizationFilter, + Filter::ExternalLinkFilter, + Filter::PlantumlFilter + ] + end + end + end +end diff --git a/lib/gitlab/asciidoc.rb b/lib/gitlab/asciidoc.rb index fba80c7132e..96d38f6daa0 100644 --- a/lib/gitlab/asciidoc.rb +++ b/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 diff --git a/lib/gitlab/other_markup.rb b/lib/gitlab/other_markup.rb index c2adc9aa10b..31a24460f0f 100644 --- a/lib/gitlab/other_markup.rb +++ b/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) + context[:pipeline] = :markup - filter = Banzai::Filter::SanitizationFilter.new(html) - html = filter.call.to_s + html = Banzai.render(html, context) html.html_safe end diff --git a/spec/lib/gitlab/asciidoc_spec.rb b/spec/lib/gitlab/asciidoc_spec.rb index 0f47fb2fbd9..f284dd14cec 100644 --- a/spec/lib/gitlab/asciidoc_spec.rb +++ b/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) diff --git a/spec/lib/gitlab/other_markup_spec.rb b/spec/lib/gitlab/other_markup_spec.rb index d6d53e8586c..c0f5fa9dc1f 100644 --- a/spec/lib/gitlab/other_markup_spec.rb +++ b/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 |