Skip to content

Commit

Permalink
Fix rendering <a> without href when scheme unsupported
Browse files Browse the repository at this point in the history
- Disallow links with relative paths
- Disallow iframes with non-http protocols and relative paths

Close #13037
  • Loading branch information
Gargron committed Feb 8, 2020
1 parent aeb6efb commit 1b5ca22
Show file tree
Hide file tree
Showing 2 changed files with 55 additions and 6 deletions.
45 changes: 39 additions & 6 deletions app/lib/sanitize_config.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,23 @@

class Sanitize
module Config
HTTP_PROTOCOLS ||= ['http', 'https', 'dat', 'dweb', 'ipfs', 'ipns', 'ssb', 'gopher', 'xmpp', 'magnet', :relative].freeze
HTTP_PROTOCOLS = %w(
http
https
).freeze

LINK_PROTOCOLS = %w(
http
https
dat
dweb
ipfs
ipns
ssb
gopher
xmpp
magnet
).freeze

CLASS_WHITELIST_TRANSFORMER = lambda do |env|
node = env[:node]
Expand All @@ -19,19 +35,37 @@ module Config
node['class'] = class_list.join(' ')
end

UNSUPPORTED_HREF_TRANSFORMER = lambda do |env|
return unless env[:node_name] == 'a'

current_node = env[:node]

scheme = begin
if current_node['href'] =~ Sanitize::REGEX_PROTOCOL
Regexp.last_match(1).downcase
else
:relative
end
end

current_node.replace(current_node.text) unless LINK_PROTOCOLS.include?(scheme)
end

UNSUPPORTED_ELEMENTS_TRANSFORMER = lambda do |env|
return unless %w(h1 h2 h3 h4 h5 h6 blockquote pre ul ol li).include?(env[:node_name])

current_node = env[:node]

case env[:node_name]
when 'li'
env[:node].traverse do |node|
current_node.traverse do |node|
next unless %w(p ul ol li).include?(node.name)

node.add_next_sibling('<br>') if node.next_sibling
node.replace(node.children) unless node.text?
end
else
env[:node].name = 'p'
current_node.name = 'p'
end
end

Expand All @@ -50,13 +84,12 @@ module Config
},
},

protocols: {
'a' => { 'href' => HTTP_PROTOCOLS },
},
protocols: {},

transformers: [
CLASS_WHITELIST_TRANSFORMER,
UNSUPPORTED_ELEMENTS_TRANSFORMER,
UNSUPPORTED_HREF_TRANSFORMER,
]
)

Expand Down
16 changes: 16 additions & 0 deletions spec/lib/sanitize_config_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,5 +26,21 @@
it 'keep links in lists' do
expect(Sanitize.fragment('<p>Check out:</p><ul><li><a href="https://joinmastodon.org" rel="nofollow noopener noreferrer" target="_blank">joinmastodon.org</a></li><li>Bar</li></ul>', subject)).to eq '<p>Check out:</p><p><a href="https://joinmastodon.org" rel="nofollow noopener noreferrer" target="_blank">joinmastodon.org</a><br>Bar</p>'
end

it 'removes a without href' do
expect(Sanitize.fragment('<a>Test</a>', subject)).to eq 'Test'
end

it 'removes a without href and only keeps text content' do
expect(Sanitize.fragment('<a><span class="invisible">foo&amp;</span><span>Test</span></a>', subject)).to eq 'foo&amp;Test'
end

it 'removes a with unsupported scheme in href' do
expect(Sanitize.fragment('<a href="foo://bar">Test</a>', subject)).to eq 'Test'
end

it 'keeps a with href' do
expect(Sanitize.fragment('<a href="http://example.com">Test</a>', subject)).to eq '<a href="http://example.com" rel="nofollow noopener noreferrer" target="_blank">Test</a>'
end
end
end

0 comments on commit 1b5ca22

Please sign in to comment.