Commit 3699eea0 authored by Rémy Coutable's avatar Rémy Coutable

Merge branch 'nicolasdular/allow-broadcast-message-styling' into 'master'

Allow styling links in broadcast messages

See merge request gitlab-org/gitlab!21522
parents 0a8f611b c75c21b1
---
title: Allow styling broadcast messages
merge_request: 21522
author:
type: added
......@@ -7,6 +7,7 @@ module Banzai
#
# - Banzai::Filter::SanitizationFilter (Markdown)
# - Banzai::Filter::AsciiDocSanitizationFilter (AsciiDoc/Asciidoctor)
# - Banzai::Filter::BroadcastMessageSanitizationFilter (Markdown with styled links and line breaks)
#
# Extends HTML::Pipeline::SanitizationFilter with common rules.
class BaseSanitizationFilter < HTML::Pipeline::SanitizationFilter
......
# frozen_string_literal: true
module Banzai
module Filter
# Sanitize HTML produced by Markdown. Allows styling of links and usage of line breaks.
#
# Extends Banzai::Filter::BaseSanitizationFilter with specific rules.
class BroadcastMessageSanitizationFilter < Banzai::Filter::BaseSanitizationFilter
def customize_whitelist(whitelist)
whitelist[:elements].push('br')
whitelist[:attributes]['a'].push('class', 'style')
whitelist[:css] = { properties: %w(color border background padding margin text-decoration) }
whitelist
end
end
end
end
......@@ -6,7 +6,7 @@ module Banzai
def self.filters
@filters ||= FilterArray[
Filter::MarkdownFilter,
Filter::SanitizationFilter,
Filter::BroadcastMessageSanitizationFilter,
Filter::EmojiFilter,
Filter::ColorFilter,
......
# frozen_string_literal: true
require 'spec_helper'
describe Banzai::Filter::BroadcastMessageSanitizationFilter do
include FilterSpecHelper
it_behaves_like 'default whitelist'
describe 'custom whitelist' do
it_behaves_like 'XSS prevention'
it_behaves_like 'sanitize link'
subject { filter(exp).to_html }
context 'allows `a` elements' do
let(:exp) { %q{<a href="/">Link</a>} }
it { is_expected.to eq(exp) }
end
context 'allows `br` elements' do
let(:exp) { %q{Hello<br>World} }
it { is_expected.to eq(exp) }
end
context 'when `a` elements have `style` attribute' do
let(:whitelisted_style) { 'color: red; border: blue; background: green; padding: 10px; margin: 10px; text-decoration: underline;' }
context 'allows specific properties' do
let(:exp) { %{<a href="#" style="#{whitelisted_style}">Stylish Link</a>} }
it { is_expected.to eq(exp) }
end
it 'disallows other properties in `style` attribute on `a` elements' do
style = [whitelisted_style, 'position: fixed'].join(';')
doc = filter(%{<a href="#" style="#{style}">Stylish Link</a>})
expect(doc.at_css('a')['style']).to eq(whitelisted_style)
end
end
context 'allows `class` on `a` elements' do
let(:exp) { %q{<a href="#" class="btn">Button Link</a>} }
it { is_expected.to eq(exp) }
end
end
end
......@@ -5,48 +5,12 @@ require 'spec_helper'
describe Banzai::Filter::SanitizationFilter do
include FilterSpecHelper
describe 'default whitelist' do
it 'sanitizes tags that are not whitelisted' do
act = %q{<textarea>no inputs</textarea> and <blink>no blinks</blink>}
exp = 'no inputs and no blinks'
expect(filter(act).to_html).to eq exp
end
it 'sanitizes tag attributes' do
act = %q{<a href="http://example.com/bar.html" onclick="bar">Text</a>}
exp = %q{<a href="http://example.com/bar.html">Text</a>}
expect(filter(act).to_html).to eq exp
end
it 'sanitizes javascript in attributes' do
act = %q(<a href="javascript:alert('foo')">Text</a>)
exp = '<a>Text</a>'
expect(filter(act).to_html).to eq exp
end
it 'sanitizes mixed-cased javascript in attributes' do
act = %q(<a href="javaScript:alert('foo')">Text</a>)
exp = '<a>Text</a>'
expect(filter(act).to_html).to eq exp
end
it 'allows whitelisted HTML tags from the user' do
exp = act = "<dl>\n<dt>Term</dt>\n<dd>Definition</dd>\n</dl>"
expect(filter(act).to_html).to eq exp
end
it 'sanitizes `class` attribute on any element' do
act = %q{<strong class="foo">Strong</strong>}
expect(filter(act).to_html).to eq %q{<strong>Strong</strong>}
end
it 'sanitizes `id` attribute on any element' do
act = %q{<em id="foo">Emphasis</em>}
expect(filter(act).to_html).to eq %q{<em>Emphasis</em>}
end
end
it_behaves_like 'default whitelist'
describe 'custom whitelist' do
it_behaves_like 'XSS prevention'
it_behaves_like 'sanitize link'
it 'customizes the whitelist only once' do
instance = described_class.new('Foo')
control_count = instance.whitelist[:transformers].size
......@@ -167,142 +131,6 @@ describe Banzai::Filter::SanitizationFilter do
expect(filter(html).to_html).to eq(output)
end
it 'removes `rel` attribute from `a` elements' do
act = %q{<a href="#" rel="nofollow">Link</a>}
exp = %q{<a href="#">Link</a>}
expect(filter(act).to_html).to eq exp
end
# Adapted from the Sanitize test suite: http://git.io/vczrM
protocols = {
'protocol-based JS injection: simple, no spaces' => {
input: '<a href="javascript:alert(\'XSS\');">foo</a>',
output: '<a>foo</a>'
},
'protocol-based JS injection: simple, spaces before' => {
input: '<a href="javascript :alert(\'XSS\');">foo</a>',
output: '<a>foo</a>'
},
'protocol-based JS injection: simple, spaces after' => {
input: '<a href="javascript: alert(\'XSS\');">foo</a>',
output: '<a>foo</a>'
},
'protocol-based JS injection: simple, spaces before and after' => {
input: '<a href="javascript : alert(\'XSS\');">foo</a>',
output: '<a>foo</a>'
},
'protocol-based JS injection: preceding colon' => {
input: '<a href=":javascript:alert(\'XSS\');">foo</a>',
output: '<a>foo</a>'
},
'protocol-based JS injection: UTF-8 encoding' => {
input: '<a href="javascript&#58;">foo</a>',
output: '<a>foo</a>'
},
'protocol-based JS injection: long UTF-8 encoding' => {
input: '<a href="javascript&#0058;">foo</a>',
output: '<a>foo</a>'
},
'protocol-based JS injection: long UTF-8 encoding without semicolons' => {
input: '<a href=&#0000106&#0000097&#0000118&#0000097&#0000115&#0000099&#0000114&#0000105&#0000112&#0000116&#0000058&#0000097&#0000108&#0000101&#0000114&#0000116&#0000040&#0000039&#0000088&#0000083&#0000083&#0000039&#0000041>foo</a>',
output: '<a>foo</a>'
},
'protocol-based JS injection: hex encoding' => {
input: '<a href="javascript&#x3A;">foo</a>',
output: '<a>foo</a>'
},
'protocol-based JS injection: long hex encoding' => {
input: '<a href="javascript&#x003A;">foo</a>',
output: '<a>foo</a>'
},
'protocol-based JS injection: hex encoding without semicolons' => {
input: '<a href=&#x6A&#x61&#x76&#x61&#x73&#x63&#x72&#x69&#x70&#x74&#x3A&#x61&#x6C&#x65&#x72&#x74&#x28&#x27&#x58&#x53&#x53&#x27&#x29>foo</a>',
output: '<a>foo</a>'
},
'protocol-based JS injection: null char' => {
input: "<a href=java\0script:alert(\"XSS\")>foo</a>",
output: '<a href="java"></a>'
},
'protocol-based JS injection: invalid URL char' => {
input: '<img src=java\script:alert("XSS")>',
output: '<img>'
},
'protocol-based JS injection: Unicode' => {
input: %Q(<a href="\u0001java\u0003script:alert('XSS')">foo</a>),
output: '<a>foo</a>'
},
'protocol-based JS injection: spaces and entities' => {
input: '<a href=" &#14; javascript:alert(\'XSS\');">foo</a>',
output: '<a href="">foo</a>'
},
'protocol whitespace' => {
input: '<a href=" http://example.com/"></a>',
output: '<a href="http://example.com/"></a>'
}
}
protocols.each do |name, data|
it "disallows #{name}" do
doc = filter(data[:input])
expect(doc.to_html).to eq data[:output]
end
end
it 'disallows data links' do
input = '<a href="data:text/html;base64,PHNjcmlwdD5hbGVydCgnWFNTJyk8L3NjcmlwdD4K">XSS</a>'
output = filter(input)
expect(output.to_html).to eq '<a>XSS</a>'
end
it 'disallows vbscript links' do
input = '<a href="vbscript:alert(document.domain)">XSS</a>'
output = filter(input)
expect(output.to_html).to eq '<a>XSS</a>'
end
it 'disallows invalid URIs' do
expect(Addressable::URI).to receive(:parse).with('foo://example.com')
.and_raise(Addressable::URI::InvalidURIError)
input = '<a href="foo://example.com">Foo</a>'
output = filter(input)
expect(output.to_html).to eq '<a>Foo</a>'
end
it 'allows non-standard anchor schemes' do
exp = %q{<a href="irc://irc.freenode.net/git">IRC</a>}
act = filter(exp)
expect(act.to_html).to eq exp
end
it 'allows relative links' do
exp = %q{<a href="foo/bar.md">foo/bar.md</a>}
act = filter(exp)
expect(act.to_html).to eq exp
end
it 'allows the `data-sourcepos` attribute globally' do
exp = %q{<p data-sourcepos="1:1-1:10">foo/bar.md</p>}
act = filter(exp)
......
# frozen_string_literal: true
require 'spec_helper'
describe Banzai::Pipeline::BroadcastMessagePipeline do
before do
stub_commonmark_sourcepos_disabled
end
subject { described_class.to_html(exp, project: spy) }
context "allows `a` elements" do
let(:exp) { "<a>Link</a>" }
it { is_expected.to eq("<p>#{exp}</p>") }
end
context "allows `br` elements" do
let(:exp) { "Hello<br>World" }
it { is_expected.to eq("<p>#{exp}</p>") }
end
end
# frozen_string_literal: true
RSpec.shared_examples 'default whitelist' do
it 'sanitizes tags that are not whitelisted' do
act = %q{<textarea>no inputs</textarea> and <blink>no blinks</blink>}
exp = 'no inputs and no blinks'
expect(filter(act).to_html).to eq exp
end
it 'sanitizes tag attributes' do
act = %q{<a href="http://example.com/bar.html" onclick="bar">Text</a>}
exp = %q{<a href="http://example.com/bar.html">Text</a>}
expect(filter(act).to_html).to eq exp
end
it 'sanitizes javascript in attributes' do
act = %q(<a href="javascript:alert('foo')">Text</a>)
exp = '<a>Text</a>'
expect(filter(act).to_html).to eq exp
end
it 'sanitizes mixed-cased javascript in attributes' do
act = %q(<a href="javaScript:alert('foo')">Text</a>)
exp = '<a>Text</a>'
expect(filter(act).to_html).to eq exp
end
it 'allows whitelisted HTML tags from the user' do
exp = act = "<dl>\n<dt>Term</dt>\n<dd>Definition</dd>\n</dl>"
expect(filter(act).to_html).to eq exp
end
it 'sanitizes `class` attribute on any element' do
act = %q{<strong class="foo">Strong</strong>}
expect(filter(act).to_html).to eq %q{<strong>Strong</strong>}
end
it 'sanitizes `id` attribute on any element' do
act = %q{<em id="foo">Emphasis</em>}
expect(filter(act).to_html).to eq %q{<em>Emphasis</em>}
end
end
RSpec.shared_examples 'XSS prevention' do
# Adapted from the Sanitize test suite: http://git.io/vczrM
protocols = {
'protocol-based JS injection: simple, no spaces' => {
input: '<a href="javascript:alert(\'XSS\');">foo</a>',
output: '<a>foo</a>'
},
'protocol-based JS injection: simple, spaces before' => {
input: '<a href="javascript :alert(\'XSS\');">foo</a>',
output: '<a>foo</a>'
},
'protocol-based JS injection: simple, spaces after' => {
input: '<a href="javascript: alert(\'XSS\');">foo</a>',
output: '<a>foo</a>'
},
'protocol-based JS injection: simple, spaces before and after' => {
input: '<a href="javascript : alert(\'XSS\');">foo</a>',
output: '<a>foo</a>'
},
'protocol-based JS injection: preceding colon' => {
input: '<a href=":javascript:alert(\'XSS\');">foo</a>',
output: '<a>foo</a>'
},
'protocol-based JS injection: UTF-8 encoding' => {
input: '<a href="javascript&#58;">foo</a>',
output: '<a>foo</a>'
},
'protocol-based JS injection: long UTF-8 encoding' => {
input: '<a href="javascript&#0058;">foo</a>',
output: '<a>foo</a>'
},
'protocol-based JS injection: long UTF-8 encoding without semicolons' => {
input: '<a href=&#0000106&#0000097&#0000118&#0000097&#0000115&#0000099&#0000114&#0000105&#0000112&#0000116&#0000058&#0000097&#0000108&#0000101&#0000114&#0000116&#0000040&#0000039&#0000088&#0000083&#0000083&#0000039&#0000041>foo</a>',
output: '<a>foo</a>'
},
'protocol-based JS injection: hex encoding' => {
input: '<a href="javascript&#x3A;">foo</a>',
output: '<a>foo</a>'
},
'protocol-based JS injection: long hex encoding' => {
input: '<a href="javascript&#x003A;">foo</a>',
output: '<a>foo</a>'
},
'protocol-based JS injection: hex encoding without semicolons' => {
input: '<a href=&#x6A&#x61&#x76&#x61&#x73&#x63&#x72&#x69&#x70&#x74&#x3A&#x61&#x6C&#x65&#x72&#x74&#x28&#x27&#x58&#x53&#x53&#x27&#x29>foo</a>',
output: '<a>foo</a>'
},
'protocol-based JS injection: null char' => {
input: "<a href=java\0script:alert(\"XSS\")>foo</a>",
output: '<a href="java"></a>'
},
'protocol-based JS injection: invalid URL char' => {
input: '<img src=java\script:alert("XSS")>',
output: '<img>'
},
'protocol-based JS injection: Unicode' => {
input: %Q(<a href="\u0001java\u0003script:alert('XSS')">foo</a>),
output: '<a>foo</a>'
},
'protocol-based JS injection: spaces and entities' => {
input: '<a href=" &#14; javascript:alert(\'XSS\');">foo</a>',
output: '<a href="">foo</a>'
},
'protocol whitespace' => {
input: '<a href=" http://example.com/"></a>',
output: '<a href="http://example.com/"></a>'
}
}
protocols.each do |name, data|
it "disallows #{name}" do
doc = filter(data[:input])
expect(doc.to_html).to eq data[:output]
end
end
it 'disallows data links' do
input = '<a href="data:text/html;base64,PHNjcmlwdD5hbGVydCgnWFNTJyk8L3NjcmlwdD4K">XSS</a>'
output = filter(input)
expect(output.to_html).to eq '<a>XSS</a>'
end
it 'disallows vbscript links' do
input = '<a href="vbscript:alert(document.domain)">XSS</a>'
output = filter(input)
expect(output.to_html).to eq '<a>XSS</a>'
end
end
RSpec.shared_examples 'sanitize link' do
it 'removes `rel` attribute from `a` elements' do
act = %q{<a href="#" rel="nofollow">Link</a>}
exp = %q{<a href="#">Link</a>}
expect(filter(act).to_html).to eq exp
end
it 'disallows invalid URIs' do
expect(Addressable::URI).to receive(:parse).with('foo://example.com')
.and_raise(Addressable::URI::InvalidURIError)
input = '<a href="foo://example.com">Foo</a>'
output = filter(input)
expect(output.to_html).to eq '<a>Foo</a>'
end
it 'allows non-standard anchor schemes' do
exp = %q{<a href="irc://irc.freenode.net/git">IRC</a>}
act = filter(exp)
expect(act.to_html).to eq exp
end
it 'allows relative links' do
exp = %q{<a href="foo/bar.md">foo/bar.md</a>}
act = filter(exp)
expect(act.to_html).to eq exp
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