Commit cd391b66 authored by Douwe Maan's avatar Douwe Maan

Merge branch 'rs-data-links' into 'master'

Sanitize `data` and `vbscript` links

Closes #13625 

Closes https://dev.gitlab.org/gitlab/gitlabhq/issues/2660

See merge request !2905
parents 2ea8f71b 989946f3
...@@ -7,6 +7,8 @@ module Banzai ...@@ -7,6 +7,8 @@ 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
def whitelist def whitelist
whitelist = super whitelist = super
...@@ -43,8 +45,8 @@ module Banzai ...@@ -43,8 +45,8 @@ module Banzai
# Allow any protocol in `a` elements... # Allow any protocol in `a` elements...
whitelist[:protocols].delete('a') whitelist[:protocols].delete('a')
# ...but then remove links with the `javascript` protocol # ...but then remove links with unsafe protocols
whitelist[:transformers].push(remove_javascript_links) whitelist[:transformers].push(remove_unsafe_links)
# Remove `rel` attribute from `a` elements # Remove `rel` attribute from `a` elements
whitelist[:transformers].push(remove_rel) whitelist[:transformers].push(remove_rel)
...@@ -55,14 +57,14 @@ module Banzai ...@@ -55,14 +57,14 @@ module Banzai
whitelist whitelist
end end
def remove_javascript_links def remove_unsafe_links
lambda do |env| lambda do |env|
node = env[:node] node = env[:node]
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?('javascript', ':javascript') if node['href'].start_with?(*UNSAFE_PROTOCOLS)
node.remove_attribute('href') node.remove_attribute('href')
end end
end end
......
...@@ -156,13 +156,27 @@ describe Banzai::Filter::SanitizationFilter, lib: true do ...@@ -156,13 +156,27 @@ describe Banzai::Filter::SanitizationFilter, lib: true do
} }
protocols.each do |name, data| protocols.each do |name, data|
it "handles #{name}" do it "disallows #{name}" do
doc = filter(data[:input]) doc = filter(data[:input])
expect(doc.to_html).to eq data[:output] expect(doc.to_html).to eq data[:output]
end end
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 '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