Add custom protocol whitelisting to SanitizationFilter
Addresses internal https://dev.gitlab.org/gitlab/gitlabhq/issues/2613
This commit is contained in:
parent
d7eceafb27
commit
16f8ca566b
2 changed files with 110 additions and 10 deletions
|
@ -48,6 +48,12 @@ module Gitlab
|
||||||
# Allow span elements
|
# Allow span elements
|
||||||
whitelist[:elements].push('span')
|
whitelist[:elements].push('span')
|
||||||
|
|
||||||
|
# Allow any protocol in `a` elements...
|
||||||
|
whitelist[:protocols].delete('a')
|
||||||
|
|
||||||
|
# ...but then remove links with the `javascript` protocol
|
||||||
|
whitelist[:transformers].push(remove_javascript_links)
|
||||||
|
|
||||||
# Remove `rel` attribute from `a` elements
|
# Remove `rel` attribute from `a` elements
|
||||||
whitelist[:transformers].push(remove_rel)
|
whitelist[:transformers].push(remove_rel)
|
||||||
|
|
||||||
|
@ -57,6 +63,19 @@ module Gitlab
|
||||||
whitelist
|
whitelist
|
||||||
end
|
end
|
||||||
|
|
||||||
|
def remove_javascript_links
|
||||||
|
lambda do |env|
|
||||||
|
node = env[:node]
|
||||||
|
|
||||||
|
return unless node.name == 'a'
|
||||||
|
return unless node.has_attribute?('href')
|
||||||
|
|
||||||
|
if node['href'].start_with?('javascript', ':javascript')
|
||||||
|
node.remove_attribute('href')
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
def remove_rel
|
def remove_rel
|
||||||
lambda do |env|
|
lambda do |env|
|
||||||
if env[:node_name] == 'a'
|
if env[:node_name] == 'a'
|
||||||
|
|
|
@ -44,7 +44,7 @@ module Gitlab::Markdown
|
||||||
instance = described_class.new('Foo')
|
instance = described_class.new('Foo')
|
||||||
3.times { instance.whitelist }
|
3.times { instance.whitelist }
|
||||||
|
|
||||||
expect(instance.whitelist[:transformers].size).to eq 4
|
expect(instance.whitelist[:transformers].size).to eq 5
|
||||||
end
|
end
|
||||||
|
|
||||||
it 'allows syntax highlighting' do
|
it 'allows syntax highlighting' do
|
||||||
|
@ -77,19 +77,100 @@ module Gitlab::Markdown
|
||||||
end
|
end
|
||||||
|
|
||||||
it 'removes `rel` attribute from `a` elements' do
|
it 'removes `rel` attribute from `a` elements' do
|
||||||
doc = filter(%q{<a href="#" rel="nofollow">Link</a>})
|
act = %q{<a href="#" rel="nofollow">Link</a>}
|
||||||
|
exp = %q{<a href="#">Link</a>}
|
||||||
|
|
||||||
expect(doc.css('a').size).to eq 1
|
expect(filter(act).to_html).to eq exp
|
||||||
expect(doc.at_css('a')['href']).to eq '#'
|
|
||||||
expect(doc.at_css('a')['rel']).to be_nil
|
|
||||||
end
|
end
|
||||||
|
|
||||||
it 'removes script-like `href` attribute from `a` elements' do
|
# Adapted from the Sanitize test suite: http://git.io/vczrM
|
||||||
html = %q{<a href="javascript:alert('Hi')">Hi</a>}
|
protocols = {
|
||||||
doc = filter(html)
|
'protocol-based JS injection: simple, no spaces' => {
|
||||||
|
input: '<a href="javascript:alert(\'XSS\');">foo</a>',
|
||||||
|
output: '<a>foo</a>'
|
||||||
|
},
|
||||||
|
|
||||||
expect(doc.css('a').size).to eq 1
|
'protocol-based JS injection: simple, spaces before' => {
|
||||||
expect(doc.at_css('a')['href']).to be_nil
|
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:">foo</a>',
|
||||||
|
output: '<a>foo</a>'
|
||||||
|
},
|
||||||
|
|
||||||
|
'protocol-based JS injection: long UTF-8 encoding' => {
|
||||||
|
input: '<a href="javascript:">foo</a>',
|
||||||
|
output: '<a>foo</a>'
|
||||||
|
},
|
||||||
|
|
||||||
|
'protocol-based JS injection: long UTF-8 encoding without semicolons' => {
|
||||||
|
input: '<a href=javascript:alert('XSS')>foo</a>',
|
||||||
|
output: '<a>foo</a>'
|
||||||
|
},
|
||||||
|
|
||||||
|
'protocol-based JS injection: hex encoding' => {
|
||||||
|
input: '<a href="javascript:">foo</a>',
|
||||||
|
output: '<a>foo</a>'
|
||||||
|
},
|
||||||
|
|
||||||
|
'protocol-based JS injection: long hex encoding' => {
|
||||||
|
input: '<a href="javascript:">foo</a>',
|
||||||
|
output: '<a>foo</a>'
|
||||||
|
},
|
||||||
|
|
||||||
|
'protocol-based JS injection: hex encoding without semicolons' => {
|
||||||
|
input: '<a href=javascript:alert('XSS')>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: spaces and entities' => {
|
||||||
|
input: '<a href="  javascript:alert(\'XSS\');">foo</a>',
|
||||||
|
output: '<a href="">foo</a>'
|
||||||
|
},
|
||||||
|
}
|
||||||
|
|
||||||
|
protocols.each do |name, data|
|
||||||
|
it "handles #{name}" do
|
||||||
|
doc = filter(data[:input])
|
||||||
|
|
||||||
|
expect(doc.to_html).to eq data[:output]
|
||||||
|
end
|
||||||
|
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
|
||||||
end
|
end
|
||||||
|
|
||||||
|
|
Loading…
Reference in a new issue