Commit a60e09d3 authored by Douwe Maan's avatar Douwe Maan

Merge branch 'rs-issue-14220' into 'master'

Be more intelligent about sanitizing links with unsafe protocols

This prevents false matches on relative links like
`[database](database.md)`.

Closes https://gitlab.com/gitlab-org/gitlab-ce/issues/14220

See merge request !3210
parents c4b35a62 41de7b34
...@@ -7,7 +7,7 @@ module Banzai ...@@ -7,7 +7,7 @@ module Banzai
# #
# Extends HTML::Pipeline::SanitizationFilter with a custom whitelist. # Extends HTML::Pipeline::SanitizationFilter with a custom whitelist.
class SanitizationFilter < HTML::Pipeline::SanitizationFilter class SanitizationFilter < HTML::Pipeline::SanitizationFilter
UNSAFE_PROTOCOLS = %w(javascript :javascript data vbscript).freeze UNSAFE_PROTOCOLS = %w(data javascript vbscript).freeze
def whitelist def whitelist
whitelist = super whitelist = super
...@@ -64,7 +64,12 @@ module Banzai ...@@ -64,7 +64,12 @@ module Banzai
return unless node.name == 'a' return unless node.name == 'a'
return unless node.has_attribute?('href') return unless node.has_attribute?('href')
if node['href'].start_with?(*UNSAFE_PROTOCOLS) begin
uri = Addressable::URI.parse(node['href'])
uri.scheme.strip! if uri.scheme
node.remove_attribute('href') if UNSAFE_PROTOCOLS.include?(uri.scheme)
rescue Addressable::URI::InvalidURIError
node.remove_attribute('href') node.remove_attribute('href')
end end
end end
......
...@@ -149,10 +149,20 @@ describe Banzai::Filter::SanitizationFilter, lib: true do ...@@ -149,10 +149,20 @@ describe Banzai::Filter::SanitizationFilter, lib: true do
output: '<a href="java"></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: spaces and entities' => { 'protocol-based JS injection: spaces and entities' => {
input: '<a href=" &#14; javascript:alert(\'XSS\');">foo</a>', input: '<a href=" &#14; javascript:alert(\'XSS\');">foo</a>',
output: '<a href="">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| protocols.each do |name, data|
...@@ -177,6 +187,16 @@ describe Banzai::Filter::SanitizationFilter, lib: true do ...@@ -177,6 +187,16 @@ describe Banzai::Filter::SanitizationFilter, lib: true do
expect(output.to_html).to eq '<a>XSS</a>' expect(output.to_html).to eq '<a>XSS</a>'
end 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 it 'allows non-standard anchor schemes' do
exp = %q{<a href="irc://irc.freenode.net/git">IRC</a>} exp = %q{<a href="irc://irc.freenode.net/git">IRC</a>}
act = filter(exp) act = filter(exp)
......
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