diff options
author | Brett Walker <bwalker@gitlab.com> | 2019-01-14 16:57:54 -0600 |
---|---|---|
committer | Yorick Peterse <yorickpeterse@gmail.com> | 2019-01-31 16:52:20 +0100 |
commit | 40983f4a9c960bac4eb59a54816ae63177015c51 (patch) | |
tree | 6d4a085affdd9af4dd351f4b69b044549b93da28 /spec/lib/banzai | |
parent | b3c13bbb3c62c90dbb9a606b27699df8d681cec3 (diff) | |
download | gitlab-ce-40983f4a9c960bac4eb59a54816ae63177015c51.tar.gz |
Show tooltip for malicious looking links
Such as those with IDN homographs or embedded
right-to-left (RTLO) characters.
Autolinked hrefs should be escaped
Diffstat (limited to 'spec/lib/banzai')
-rw-r--r-- | spec/lib/banzai/filter/autolink_filter_spec.rb | 16 | ||||
-rw-r--r-- | spec/lib/banzai/filter/external_link_filter_spec.rb | 65 | ||||
-rw-r--r-- | spec/lib/banzai/pipeline/email_pipeline_spec.rb | 14 | ||||
-rw-r--r-- | spec/lib/banzai/pipeline/full_pipeline_spec.rb | 38 |
4 files changed, 133 insertions, 0 deletions
diff --git a/spec/lib/banzai/filter/autolink_filter_spec.rb b/spec/lib/banzai/filter/autolink_filter_spec.rb index 7a457403b51..6217381c491 100644 --- a/spec/lib/banzai/filter/autolink_filter_spec.rb +++ b/spec/lib/banzai/filter/autolink_filter_spec.rb @@ -188,6 +188,22 @@ describe Banzai::Filter::AutolinkFilter do expect(doc.at_css('a')['class']).to eq 'custom' end + it 'escapes RTLO and other characters' do + # rendered text looks like "http://example.com/evilexe.mp3" + evil_link = "#{link}evil\u202E3pm.exe" + doc = filter("#{evil_link}") + + expect(doc.at_css('a')['href']).to eq "http://about.gitlab.com/evil%E2%80%AE3pm.exe" + end + + it 'encodes international domains' do + link = "http://one😄two.com" + expected = "http://one%F0%9F%98%84two.com" + doc = filter(link) + + expect(doc.at_css('a')['href']).to eq expected + end + described_class::IGNORE_PARENTS.each do |elem| it "ignores valid links contained inside '#{elem}' element" do exp = act = "<#{elem}>See #{link}</#{elem}>" diff --git a/spec/lib/banzai/filter/external_link_filter_spec.rb b/spec/lib/banzai/filter/external_link_filter_spec.rb index e6dae8d5382..2acbe05f082 100644 --- a/spec/lib/banzai/filter/external_link_filter_spec.rb +++ b/spec/lib/banzai/filter/external_link_filter_spec.rb @@ -62,6 +62,13 @@ describe Banzai::Filter::ExternalLinkFilter do expect(doc.to_html).to eq(expected) end + + it 'adds rel and target to improperly formatted autolinks' do + doc = filter %q(<p><a href="mailto://jblogs@example.com">mailto://jblogs@example.com</a></p>) + expected = %q(<p><a href="mailto://jblogs@example.com" rel="nofollow noreferrer noopener" target="_blank">mailto://jblogs@example.com</a></p>) + + expect(doc.to_html).to eq(expected) + end end context 'for links with a username' do @@ -112,4 +119,62 @@ describe Banzai::Filter::ExternalLinkFilter do it_behaves_like 'an external link with rel attribute' end + + context 'links with RTLO character' do + # In rendered text this looks like "http://example.com/evilexe.mp3" + let(:doc) { filter %Q(<a href="http://example.com/evil%E2%80%AE3pm.exe">http://example.com/evil\u202E3pm.exe</a>) } + + it_behaves_like 'an external link with rel attribute' + + it 'escapes RTLO in link text' do + expected = %q(http://example.com/evil%E2%80%AE3pm.exe</a>) + + expect(doc.to_html).to include(expected) + end + + it 'does not mangle the link text' do + doc = filter %Q(<a href="http://example.com">One<span>and</span>\u202Eexe.mp3</a>) + + expect(doc.to_html).to include('One<span>and</span>%E2%80%AEexe.mp3</a>') + end + end + + context 'for generated autolinks' do + context 'with an IDN character' do + let(:doc) { filter(%q(<a href="http://exa%F0%9F%98%84mple.com">http://exa😄mple.com</a>)) } + let(:doc_email) { filter(%q(<a href="http://exa%F0%9F%98%84mple.com">http://exa😄mple.com</a>), emailable_links: true) } + + it_behaves_like 'an external link with rel attribute' + + it 'does not change the link text' do + expect(doc.to_html).to include('http://exa😄mple.com</a>') + end + + it 'uses punycode for emails' do + expect(doc_email.to_html).to include('http://xn--example-6p25f.com/</a>') + end + end + end + + context 'for links that look malicious' do + context 'with an IDN character' do + let(:doc) { filter %q(<a href="http://exa%F0%9F%98%84mple.com">http://exa😄mple.com</a>) } + + it 'adds a toolip with punycode' do + expect(doc.to_html).to include('http://exa😄mple.com</a>') + expect(doc.to_html).to include('class="has-tooltip"') + expect(doc.to_html).to include('title="http://xn--example-6p25f.com/"') + end + end + + context 'with RTLO character' do + let(:doc) { filter %q(<a href="http://example.com/evil%E2%80%AE3pm.exe">Evil Test</a>) } + + it 'adds a toolip with punycode' do + expect(doc.to_html).to include('Evil Test</a>') + expect(doc.to_html).to include('class="has-tooltip"') + expect(doc.to_html).to include('title="http://example.com/evil%E2%80%AE3pm.exe"') + end + end + end end diff --git a/spec/lib/banzai/pipeline/email_pipeline_spec.rb b/spec/lib/banzai/pipeline/email_pipeline_spec.rb index 6a11ca2f9d5..b99161109eb 100644 --- a/spec/lib/banzai/pipeline/email_pipeline_spec.rb +++ b/spec/lib/banzai/pipeline/email_pipeline_spec.rb @@ -10,5 +10,19 @@ describe Banzai::Pipeline::EmailPipeline do expect(described_class.filters).not_to be_empty expect(described_class.filters).not_to include(Banzai::Filter::ImageLazyLoadFilter) end + + it 'shows punycode for autolinks' do + examples = %W[ + http://one😄two.com + http://\u0261itlab.com + ] + + examples.each do |markdown| + result = described_class.call(markdown, project: nil)[:output] + link = result.css('a').first + + expect(link.content).to include('http://xn--') + end + end end end diff --git a/spec/lib/banzai/pipeline/full_pipeline_spec.rb b/spec/lib/banzai/pipeline/full_pipeline_spec.rb index aa503b6e1d5..3d3aa64d630 100644 --- a/spec/lib/banzai/pipeline/full_pipeline_spec.rb +++ b/spec/lib/banzai/pipeline/full_pipeline_spec.rb @@ -59,4 +59,42 @@ describe Banzai::Pipeline::FullPipeline do expect(html.lines.map(&:strip).join("\n")).to eq filtered_footnote end end + + describe 'links are detected as malicious' do + it 'has tooltips for malicious links' do + examples = %W[ + http://example.com/evil\u202E3pm.exe + [evilexe.mp3](http://example.com/evil\u202E3pm.exe) + rdar://localhost.com/\u202E3pm.exe + http://one😄two.com + [Evil-Test](http://one😄two.com) + http://\u0261itlab.com + [Evil-GitLab-link](http://\u0261itlab.com) + ![Evil-GitLab-link](http://\u0261itlab.com.png) + ] + + examples.each do |markdown| + result = described_class.call(markdown, project: nil)[:output] + link = result.css('a').first + + expect(link[:class]).to include('has-tooltip') + end + end + + it 'has no tooltips for safe links' do + examples = %w[ + http://example.com + [Safe-Test](http://example.com) + https://commons.wikimedia.org/wiki/File:اسكرام_2_-_تمنراست.jpg + [Wikipedia-link](https://commons.wikimedia.org/wiki/File:اسكرام_2_-_تمنراست.jpg) + ] + + examples.each do |markdown| + result = described_class.call(markdown, project: nil)[:output] + link = result.css('a').first + + expect(link[:class]).to be_nil + end + end + end end |