diff options
| -rw-r--r-- | changelogs/unreleased/30125-markdown-security.yml | 4 | ||||
| -rw-r--r-- | lib/banzai/filter/markdown_filter.rb | 2 | ||||
| -rw-r--r-- | lib/banzai/filter/sanitization_filter.rb | 22 | ||||
| -rw-r--r-- | lib/banzai/filter/syntax_highlight_filter.rb | 2 | ||||
| -rw-r--r-- | lib/banzai/pipeline/gfm_pipeline.rb | 2 | ||||
| -rw-r--r-- | lib/banzai/renderer/html.rb | 13 | ||||
| -rw-r--r-- | spec/helpers/events_helper_spec.rb | 27 | ||||
| -rw-r--r-- | spec/lib/banzai/filter/markdown_filter_spec.rb | 19 | ||||
| -rw-r--r-- | spec/lib/banzai/filter/sanitization_filter_spec.rb | 7 | ||||
| -rw-r--r-- | spec/lib/banzai/filter/syntax_highlight_filter_spec.rb | 6 |
10 files changed, 68 insertions, 36 deletions
diff --git a/changelogs/unreleased/30125-markdown-security.yml b/changelogs/unreleased/30125-markdown-security.yml new file mode 100644 index 00000000000..b766caf7d08 --- /dev/null +++ b/changelogs/unreleased/30125-markdown-security.yml @@ -0,0 +1,4 @@ +--- +title: Remove the class attribute from the whitelist for HTML generated from Markdown. +merge_request: +author: diff --git a/lib/banzai/filter/markdown_filter.rb b/lib/banzai/filter/markdown_filter.rb index ff580ec68f8..ee73fa91589 100644 --- a/lib/banzai/filter/markdown_filter.rb +++ b/lib/banzai/filter/markdown_filter.rb @@ -14,7 +14,7 @@ module Banzai def self.renderer @renderer ||= begin - renderer = Redcarpet::Render::HTML.new + renderer = Banzai::Renderer::HTML.new Redcarpet::Markdown.new(renderer, redcarpet_options) end end diff --git a/lib/banzai/filter/sanitization_filter.rb b/lib/banzai/filter/sanitization_filter.rb index d5f9e252f62..522217deae4 100644 --- a/lib/banzai/filter/sanitization_filter.rb +++ b/lib/banzai/filter/sanitization_filter.rb @@ -24,10 +24,6 @@ module Banzai # Only push these customizations once return if customized?(whitelist[:transformers]) - # Allow code highlighting - whitelist[:attributes]['pre'] = %w(class v-pre) - whitelist[:attributes]['span'] = %w(class) - # Allow table alignment whitelist[:attributes]['th'] = %w(style) whitelist[:attributes]['td'] = %w(style) @@ -52,9 +48,6 @@ module Banzai # Remove `rel` attribute from `a` elements whitelist[:transformers].push(self.class.remove_rel) - # Remove `class` attribute from non-highlight spans - whitelist[:transformers].push(self.class.clean_spans) - whitelist end @@ -84,21 +77,6 @@ module Banzai end end end - - def clean_spans - lambda do |env| - node = env[:node] - - return unless node.name == 'span' - return unless node.has_attribute?('class') - - unless node.ancestors.any? { |n| n.name.casecmp('pre').zero? } - node.remove_attribute('class') - end - - { node_whitelist: [node] } - end - end end end end diff --git a/lib/banzai/filter/syntax_highlight_filter.rb b/lib/banzai/filter/syntax_highlight_filter.rb index a447e2b8bff..d40d1f063bb 100644 --- a/lib/banzai/filter/syntax_highlight_filter.rb +++ b/lib/banzai/filter/syntax_highlight_filter.rb @@ -16,7 +16,7 @@ module Banzai end def highlight_node(node) - language = node.attr('class') + language = node.attr('lang') code = node.text css_classes = "code highlight" lexer = lexer_for(language) diff --git a/lib/banzai/pipeline/gfm_pipeline.rb b/lib/banzai/pipeline/gfm_pipeline.rb index b25d6f18d59..639773539ad 100644 --- a/lib/banzai/pipeline/gfm_pipeline.rb +++ b/lib/banzai/pipeline/gfm_pipeline.rb @@ -9,9 +9,9 @@ module Banzai # The GFM-to-HTML-to-GFM cycle is tested in spec/features/copy_as_gfm_spec.rb. def self.filters @filters ||= FilterArray[ - Filter::SyntaxHighlightFilter, Filter::PlantumlFilter, Filter::SanitizationFilter, + Filter::SyntaxHighlightFilter, Filter::MathFilter, Filter::UploadLinkFilter, diff --git a/lib/banzai/renderer/html.rb b/lib/banzai/renderer/html.rb new file mode 100644 index 00000000000..252caa35947 --- /dev/null +++ b/lib/banzai/renderer/html.rb @@ -0,0 +1,13 @@ +module Banzai + module Renderer + class HTML < Redcarpet::Render::HTML + def block_code(code, lang) + lang_attr = lang ? %Q{ lang="#{lang}"} : '' + + "\n<pre>" \ + "<code#{lang_attr}>#{html_escape(code)}</code>" \ + "</pre>" + end + end + end +end diff --git a/spec/helpers/events_helper_spec.rb b/spec/helpers/events_helper_spec.rb index 81ba693f2f3..0acbd584f08 100644 --- a/spec/helpers/events_helper_spec.rb +++ b/spec/helpers/events_helper_spec.rb @@ -2,8 +2,10 @@ require 'spec_helper' describe EventsHelper do describe '#event_note' do + let(:user) { build(:user) } + before do - allow(helper).to receive(:current_user).and_return(double) + allow(helper).to receive(:current_user).and_return(user) end it 'displays one line of plain text without alteration' do @@ -62,11 +64,26 @@ describe EventsHelper do expect(helper.event_note(input)).to eq(expected) end - it 'preserves style attribute within a tag' do - input = '<span class="" style="background-color: #44ad8e; color: #FFFFFF;"></span>' - expected = '<p><span style="background-color: #44ad8e; color: #FFFFFF;"></span></p>' + context 'labels formatting' do + let(:input) { 'this should be ~label_1' } - expect(helper.event_note(input)).to eq(expected) + def format_event_note(project) + create(:label, title: 'label_1', project: project) + + helper.event_note(input, { project: project }) + end + + it 'preserves style attribute for a label that can be accessed by current_user' do + project = create(:empty_project, :public) + + expect(format_event_note(project)).to match(/span class=.*style=.*/) + end + + it 'does not style a label that can not be accessed by current_user' do + project = create(:empty_project, :private) + + expect(format_event_note(project)).to eq("<p>#{input}</p>") + end end end diff --git a/spec/lib/banzai/filter/markdown_filter_spec.rb b/spec/lib/banzai/filter/markdown_filter_spec.rb new file mode 100644 index 00000000000..897288b8ad5 --- /dev/null +++ b/spec/lib/banzai/filter/markdown_filter_spec.rb @@ -0,0 +1,19 @@ +require 'spec_helper' + +describe Banzai::Filter::MarkdownFilter, lib: true do + include FilterSpecHelper + + context 'code block' do + it 'adds language to lang attribute when specified' do + result = filter("```html\nsome code\n```") + + expect(result).to start_with("\n<pre><code lang=\"html\">") + end + + it 'does not add language to lang attribute when not specified' do + result = filter("```\nsome code\n```") + + expect(result).to start_with("\n<pre><code>") + end + end +end diff --git a/spec/lib/banzai/filter/sanitization_filter_spec.rb b/spec/lib/banzai/filter/sanitization_filter_spec.rb index b4cd5f63a15..fdbc65b5e00 100644 --- a/spec/lib/banzai/filter/sanitization_filter_spec.rb +++ b/spec/lib/banzai/filter/sanitization_filter_spec.rb @@ -49,11 +49,12 @@ describe Banzai::Filter::SanitizationFilter, lib: true do instance = described_class.new('Foo') 3.times { instance.whitelist } - expect(instance.whitelist[:transformers].size).to eq 5 + expect(instance.whitelist[:transformers].size).to eq 4 end - it 'allows syntax highlighting' do - exp = act = %q{<pre class="code highlight white c"><code><span class="k">def</span></code></pre>} + it 'sanitizes `class` attribute from all elements' do + act = %q{<pre class="code highlight white c"><code><span class="k">def</span></code></pre>} + exp = %q{<pre><code><span class="k">def</span></code></pre>} expect(filter(act).to_html).to eq exp end diff --git a/spec/lib/banzai/filter/syntax_highlight_filter_spec.rb b/spec/lib/banzai/filter/syntax_highlight_filter_spec.rb index 69e3c52b35a..bfd11941867 100644 --- a/spec/lib/banzai/filter/syntax_highlight_filter_spec.rb +++ b/spec/lib/banzai/filter/syntax_highlight_filter_spec.rb @@ -12,14 +12,14 @@ describe Banzai::Filter::SyntaxHighlightFilter, lib: true do context "when a valid language is specified" do it "highlights as that language" do - result = filter('<pre><code class="ruby">def fun end</code></pre>') + result = filter('<pre><code lang="ruby">def fun end</code></pre>') expect(result.to_html).to eq('<pre class="code highlight js-syntax-highlight ruby" lang="ruby" v-pre="true"><code><span class="k">def</span> <span class="nf">fun</span> <span class="k">end</span></code></pre>') end end context "when an invalid language is specified" do it "highlights as plaintext" do - result = filter('<pre><code class="gnuplot">This is a test</code></pre>') + result = filter('<pre><code lang="gnuplot">This is a test</code></pre>') expect(result.to_html).to eq('<pre class="code highlight js-syntax-highlight plaintext" lang="plaintext" v-pre="true"><code>This is a test</code></pre>') end end @@ -30,7 +30,7 @@ describe Banzai::Filter::SyntaxHighlightFilter, lib: true do end it "highlights as plaintext" do - result = filter('<pre><code class="ruby">This is a test</code></pre>') + result = filter('<pre><code lang="ruby">This is a test</code></pre>') expect(result.to_html).to eq('<pre class="code highlight" lang="" v-pre="true"><code>This is a test</code></pre>') end end |