Commit 2e8aa209 authored by Robert Speicher's avatar Robert Speicher Committed by DJ Mountney

Merge branch '30125-markdown-security'

Remove class from SanitizationFilter whitelist

See merge request !2079
parent 5fde7c6c
---
title: Remove the class attribute from the whitelist for HTML generated from Markdown.
merge_request:
author:
......@@ -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
......
......@@ -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
......
......@@ -14,7 +14,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)
......
......@@ -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,
......
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
......@@ -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
......@@ -60,11 +62,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
......
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
......@@ -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>&lt;span class="k"&gt;def&lt;/span&gt;</code></pre>}
exp = %q{<pre><code>&lt;span class="k"&gt;def&lt;/span&gt;</code></pre>}
expect(filter(act).to_html).to eq exp
end
......
......@@ -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 id="LC1" class="line" lang="ruby"><span class="k">def</span> <span class="nf">fun</span> <span class="k">end</span></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><span id="LC1" class="line" lang="plaintext">This is a test</span></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
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment