Show tooltip for malicious looking links

Such as those with IDN homographs or embedded
right-to-left (RTLO) characters.

Autolinked hrefs should be escaped

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 e9c7a2f352e..69dc1b32c75 100644
--- a/spec/lib/banzai/pipeline/full_pipeline_spec.rb
+++ b/spec/lib/banzai/pipeline/full_pipeline_spec.rb
@@ -25,4 +25,42 @@ describe Banzai::Pipeline::FullPipeline do
       expect(result).to include(%{data-original='\"&gt;bad things'})
     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