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
This commit is contained in:
parent
b782e7c973
commit
41de7b345b
2 changed files with 27 additions and 2 deletions
|
@ -7,7 +7,7 @@ module Banzai
|
|||
#
|
||||
# Extends HTML::Pipeline::SanitizationFilter with a custom whitelist.
|
||||
class SanitizationFilter < HTML::Pipeline::SanitizationFilter
|
||||
UNSAFE_PROTOCOLS = %w(javascript :javascript data vbscript).freeze
|
||||
UNSAFE_PROTOCOLS = %w(data javascript vbscript).freeze
|
||||
|
||||
def whitelist
|
||||
whitelist = super
|
||||
|
@ -64,7 +64,12 @@ module Banzai
|
|||
return unless node.name == 'a'
|
||||
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')
|
||||
end
|
||||
end
|
||||
|
|
|
@ -149,10 +149,20 @@ describe Banzai::Filter::SanitizationFilter, lib: true do
|
|||
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' => {
|
||||
input: '<a href="  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|
|
||||
|
@ -177,6 +187,16 @@ describe Banzai::Filter::SanitizationFilter, lib: true do
|
|||
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)
|
||||
|
|
Loading…
Reference in a new issue