Sanitize ZenTao breadcrumb links

Merge branch 'security-360540-zentao-xss-links-14-10' into '14-10-stable-ee'

See merge request gitlab-org/security/gitlab!2557

Changelog: security

app/helpers/integrations_helper.rb

@@ -159,27 +159,6 @@ module IntegrationsHelper
     !Gitlab.com?
   end
 
-  def jira_issue_breadcrumb_link(issue_reference)
-    link_to '', { class: 'gl-display-flex gl-align-items-center gl-white-space-nowrap' } do
-      icon = image_tag image_path('illustrations/logos/jira.svg'), width: 15, height: 15, class: 'gl-mr-2'
-      [icon, html_escape(issue_reference)].join.html_safe
-    end
-  end
-
-  def zentao_issue_breadcrumb_link(issue)
-    link_to issue[:web_url], { target: '_blank', rel: 'noopener noreferrer', class: 'gl-display-flex gl-align-items-center gl-white-space-nowrap' } do
-      icon = image_tag image_path('logos/zentao.svg'), width: 15, height: 15, class: 'gl-mr-2'
-      [icon, html_escape(issue[:id])].join.html_safe
-    end
-  end
-
-  def zentao_issues_show_data
-    {
-      issues_show_path: project_integrations_zentao_issue_path(@project, params[:id], format: :json),
-      issues_list_path: project_integrations_zentao_issues_path(@project)
-    }
-  end
-
   extend self
 
   private

ee/app/helpers/ee/integrations_helper.rb

@@ -67,5 +67,43 @@ module EE
 
       super
     end
+
+    def jira_issue_breadcrumb_link(issue_reference)
+      external_issue_breadcrumb_link('illustrations/logos/jira.svg', issue_reference, '')
+    end
+
+    def zentao_issue_breadcrumb_link(issue)
+      external_issue_breadcrumb_link('logos/zentao.svg', issue[:id], issue[:web_url], target: '_blank')
+    end
+
+    def zentao_issues_show_data
+      {
+        issues_show_path: project_integrations_zentao_issue_path(@project, params[:id], format: :json),
+        issues_list_path: project_integrations_zentao_issues_path(@project)
+      }
+    end
+
+    private
+
+    # Use this method when dealing with issue data from external services
+    # (like Jira or ZenTao).
+    # Returns a sanitized `ActiveSupport::SafeBuffer` link.
+    def external_issue_breadcrumb_link(img, text, href, options = {})
+      icon = image_tag image_path(img), width: 15, height: 15, class: 'gl-mr-2'
+      link = sanitize(
+        link_to(
+          strip_tags(text),
+          strip_tags(href),
+          options.merge(
+            rel: 'noopener noreferrer',
+            class: 'gl-display-flex gl-align-items-center gl-white-space-nowrap'
+          )
+        ),
+        tags: %w(a img),
+        attributes: %w(target href src loading rel class width height)
+      )
+
+      [icon, link].join.html_safe
+    end
   end
 end

ee/app/views/projects/integrations/jira/issues/show.html.haml

 - add_to_breadcrumbs s_('JiraService|Jira issues'), project_integrations_jira_issues_path(@project)
 - breadcrumb_title jira_issue_breadcrumb_link(@issue_json[:references][:relative])
-- page_title html_escape(@issue_json[:title])
+- page_title @issue_json[:title]
 
 .js-jira-issues-show-app{ data: jira_issues_show_data }

ee/app/views/projects/integrations/zentao/issues/show.html.haml

 - add_to_breadcrumbs s_('ZentaoIntegration|ZenTao issues'), project_integrations_zentao_issues_path(@project)
 - breadcrumb_title zentao_issue_breadcrumb_link(@issue_json)
-- page_title html_escape(@issue_json[:title])
+- page_title @issue_json[:title]
 
 .js-zentao-issues-show-app{ data: zentao_issues_show_data }

ee/spec/helpers/ee/integrations_helper_spec.rb

@@ -106,6 +106,48 @@ RSpec.describe EE::IntegrationsHelper do
     end
   end
 
+  describe '#jira_issue_breadcrumb_link' do
+    let(:expected_html) { '<img width="15" height="15" class="gl-mr-2 lazy" data-src="/assets/illustrations/logos/jira-d90a9462f8323a5a2d9aef3c3bbb5c8a40275419aabf3cfbe6826113162b18a1.svg" src="data:image/gif;base64,R0lGODlhAQABAAAAACH5BAEKAAEALAAAAAABAAEAAAICTAEAOw==" /><a rel="noopener noreferrer" class="gl-display-flex gl-align-items-center gl-white-space-nowrap" href="">my-ref</a>' }
+
+    subject { helper.jira_issue_breadcrumb_link(issue_reference) }
+
+    context 'with a valid issue_reference' do
+      let(:issue_reference) { 'my-ref' }
+
+      it 'returns the correct HTML' do
+        is_expected.to eq(expected_html)
+      end
+    end
+
+    context 'when issue_reference contains HTML' do
+      let(:issue_reference) { "<script>alert('XSS')</script>my-ref" }
+
+      it 'strips all tags' do
+        is_expected.to eq(expected_html)
+      end
+    end
+  end
+
+  describe '#zentao_issue_breadcrumb_link' do
+    subject { helper.zentao_issue_breadcrumb_link(issue_json) }
+
+    context 'with valid issue JSON' do
+      let(:issue_json) { { id: "my-ref", web_url: "https://example.com" } }
+
+      it 'returns the correct HTML' do
+        is_expected.to eq('<img width="15" height="15" class="gl-mr-2 lazy" data-src="/assets/logos/zentao-91a4a40cfe1a1640cb4fcf645db75e0ce23fbb9984f649c0675e616d6ff8c632.svg" src="data:image/gif;base64,R0lGODlhAQABAAAAACH5BAEKAAEALAAAAAABAAEAAAICTAEAOw==" /><a target="_blank" rel="noopener noreferrer" class="gl-display-flex gl-align-items-center gl-white-space-nowrap" href="https://example.com">my-ref</a>')
+      end
+    end
+
+    context 'when issue_reference contains XSS' do
+      let(:issue_json) { { id: "<script>alert('XSS')</script>my-ref", web_url: "javascript:alert('XSS')" } }
+
+      it 'strips all tags and sanitizes' do
+        is_expected.to eq('<img width="15" height="15" class="gl-mr-2 lazy" data-src="/assets/logos/zentao-91a4a40cfe1a1640cb4fcf645db75e0ce23fbb9984f649c0675e616d6ff8c632.svg" src="data:image/gif;base64,R0lGODlhAQABAAAAACH5BAEKAAEALAAAAAABAAEAAAICTAEAOw==" /><a target="_blank" rel="noopener noreferrer" class="gl-display-flex gl-align-items-center gl-white-space-nowrap">my-ref</a>')
+      end
+    end
+  end
+
   describe '#gitlab_slack_application_data' do
     let_it_be(:projects) { create_list(:project, 3) }
 

spec/helpers/integrations_helper_spec.rb

@@ -149,19 +149,4 @@ RSpec.describe IntegrationsHelper do
       end
     end
   end
-
-  describe '#jira_issue_breadcrumb_link' do
-    let(:issue_reference) { nil }
-
-    subject { helper.jira_issue_breadcrumb_link(issue_reference) }
-
-    context 'when issue_reference contains HTML' do
-      let(:issue_reference) { "<script>alert('XSS')</script>" }
-
-      it 'escapes issue reference' do
-        is_expected.not_to include(issue_reference)
-        is_expected.to include(html_escape(issue_reference))
-      end
-    end
-  end
 end