From b0fbf001dab134b6638411f0be209bc0d1460519 Mon Sep 17 00:00:00 2001
From: Patrick Derichs <pderichs@gitlab.com>
Date: Fri, 3 May 2019 15:09:20 +0200
Subject: Fix url redaction for issue links

Add changelog entry

Add missing href to all redactor specs and removed href assignment

Remove obsolete spec

If original_content is given, it should be used for link content
---
 ...ity-fix-project-existence-disclosure-master.yml |  5 ++++
 lib/banzai/redactor.rb                             |  7 +++--
 spec/lib/banzai/redactor_spec.rb                   | 32 ++++++++++++----------
 3 files changed, 28 insertions(+), 16 deletions(-)
 create mode 100644 changelogs/unreleased/security-fix-project-existence-disclosure-master.yml

diff --git a/changelogs/unreleased/security-fix-project-existence-disclosure-master.yml b/changelogs/unreleased/security-fix-project-existence-disclosure-master.yml
new file mode 100644
index 00000000000..084439c71d9
--- /dev/null
+++ b/changelogs/unreleased/security-fix-project-existence-disclosure-master.yml
@@ -0,0 +1,5 @@
+---
+title: Fix url redaction for issue links
+merge_request:
+author:
+type: security
diff --git a/lib/banzai/redactor.rb b/lib/banzai/redactor.rb
index 7db5f5e1f7d..c2da7fec7cc 100644
--- a/lib/banzai/redactor.rb
+++ b/lib/banzai/redactor.rb
@@ -70,8 +70,11 @@ module Banzai
       # Build the raw <a> tag just with a link as href and content if
       # it's originally a link pattern. We shouldn't return a plain text href.
       original_link =
-        if link_reference == 'true' && href = original_content
-          %(<a href="#{href}">#{href}</a>)
+        if link_reference == 'true'
+          href = node.attr('href')
+          content = original_content
+
+          %(<a href="#{href}">#{content}</a>)
         end
 
       # The reference should be replaced by the original link's content,
diff --git a/spec/lib/banzai/redactor_spec.rb b/spec/lib/banzai/redactor_spec.rb
index aaeec953e4b..718649e0e10 100644
--- a/spec/lib/banzai/redactor_spec.rb
+++ b/spec/lib/banzai/redactor_spec.rb
@@ -13,10 +13,10 @@ describe Banzai::Redactor do
 
       it 'redacts an array of documents' do
         doc1 = Nokogiri::HTML
-               .fragment('<a class="gfm" data-reference-type="issue">foo</a>')
+               .fragment('<a class="gfm" href="https://www.gitlab.com" data-reference-type="issue">foo</a>')
 
         doc2 = Nokogiri::HTML
-               .fragment('<a class="gfm" data-reference-type="issue">bar</a>')
+               .fragment('<a class="gfm" href="https://www.gitlab.com" data-reference-type="issue">bar</a>')
 
         redacted_data = redactor.redact([doc1, doc2])
 
@@ -27,7 +27,7 @@ describe Banzai::Redactor do
       end
 
       it 'replaces redacted reference with inner HTML' do
-        doc = Nokogiri::HTML.fragment("<a class='gfm' data-reference-type='issue'>foo</a>")
+        doc = Nokogiri::HTML.fragment("<a class='gfm' href='https://www.gitlab.com' data-reference-type='issue'>foo</a>")
         redactor.redact([doc])
         expect(doc.to_html).to eq('foo')
       end
@@ -35,20 +35,24 @@ describe Banzai::Redactor do
       context 'when data-original attribute provided' do
         let(:original_content) { '<code>foo</code>' }
         it 'replaces redacted reference with original content' do
-          doc = Nokogiri::HTML.fragment("<a class='gfm' data-reference-type='issue' data-original='#{original_content}'>bar</a>")
+          doc = Nokogiri::HTML.fragment("<a class='gfm' href='https://www.gitlab.com' data-reference-type='issue' data-original='#{original_content}'>bar</a>")
           redactor.redact([doc])
           expect(doc.to_html).to eq(original_content)
         end
-      end
-
-      it 'returns <a> tag with original href if it is originally a link reference' do
-        href = 'http://localhost:3000'
-        doc = Nokogiri::HTML
-          .fragment("<a class='gfm' data-reference-type='issue' data-original=#{href} data-link-reference='true'>#{href}</a>")
 
-        redactor.redact([doc])
+        it 'does not replace redacted reference with original content if href is given' do
+          html = "<a href='https://www.gitlab.com' data-link-reference='true' class='gfm' data-reference-type='issue' data-reference-type='issue' data-original='Marge'>Marge</a>"
+          doc = Nokogiri::HTML.fragment(html)
+          redactor.redact([doc])
+          expect(doc.to_html).to eq('<a href="https://www.gitlab.com">Marge</a>')
+        end
 
-        expect(doc.to_html).to eq('<a href="http://localhost:3000">http://localhost:3000</a>')
+        it 'uses the original content as the link content if given' do
+          html = "<a href='https://www.gitlab.com' data-link-reference='true' class='gfm' data-reference-type='issue' data-reference-type='issue' data-original='Homer'>Marge</a>"
+          doc = Nokogiri::HTML.fragment(html)
+          redactor.redact([doc])
+          expect(doc.to_html).to eq('<a href="https://www.gitlab.com">Homer</a>')
+        end
       end
     end
 
@@ -61,7 +65,7 @@ describe Banzai::Redactor do
       end
 
       it 'redacts an issue attached' do
-        doc = Nokogiri::HTML.fragment("<a class='gfm' data-reference-type='issue' data-issue='#{issue.id}'>foo</a>")
+        doc = Nokogiri::HTML.fragment("<a class='gfm' href='https://www.gitlab.com' data-reference-type='issue' data-issue='#{issue.id}'>foo</a>")
 
         redactor.redact([doc])
 
@@ -69,7 +73,7 @@ describe Banzai::Redactor do
       end
 
       it 'redacts an external issue' do
-        doc = Nokogiri::HTML.fragment("<a class='gfm' data-reference-type='issue' data-external-issue='#{issue.id}' data-project='#{project.id}'>foo</a>")
+        doc = Nokogiri::HTML.fragment("<a class='gfm' href='https://www.gitlab.com' data-reference-type='issue' data-external-issue='#{issue.id}' data-project='#{project.id}'>foo</a>")
 
         redactor.redact([doc])
 
-- 
cgit v1.2.1