Merge branch 'security-fix-stored-xss-jira-issue-details-page' into 'master'

Fix stored XSS in the Jira issue detail pages

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

app/helpers/integrations_helper.rb

@@ -137,7 +137,7 @@ module IntegrationsHelper
   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, issue_reference].join.html_safe
+      [icon, html_escape(issue_reference)].join.html_safe
     end
   end
 

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

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

ee/spec/controllers/projects/integrations/jira/issues_controller_spec.rb

@@ -203,12 +203,12 @@ RSpec.describe Projects::Integrations::Jira::IssuesController do
       before do
         stub_licensed_features(jira_issues_integration: true)
 
-        expect_next_found_instance_of(Integrations::Jira) do |service|
-          expect(service).to receive(:find_issue).with('1', rendered_fields: true).and_return(jira_issue)
+        allow_next_found_instance_of(Integrations::Jira) do |service|
+          allow(service).to receive(:find_issue).with('1', rendered_fields: true).and_return(jira_issue)
         end
 
-        expect_next_instance_of(Integrations::JiraSerializers::IssueDetailSerializer) do |serializer|
-          expect(serializer).to receive(:represent).with(jira_issue, project: project).and_return(issue_json)
+        allow_next_instance_of(Integrations::JiraSerializers::IssueDetailSerializer) do |serializer|
+          allow(serializer).to receive(:represent).with(jira_issue, project: project).and_return(issue_json)
         end
       end
 
@@ -225,6 +225,21 @@ RSpec.describe Projects::Integrations::Jira::IssuesController do
 
         expect(json_response).to eq(issue_json)
       end
+
+      context 'when the JSON fetched from Jira contains HTML' do
+        let(:payload) { "<script>alert('XSS')</script>" }
+        let(:issue_json) { { title: payload, references: { relative: payload } } }
+
+        render_views
+
+        it 'escapes the HTML in issue titles and references' do
+          get :show, params: { namespace_id: project.namespace, project_id: project, id: 1 }
+
+          expect(response).to have_gitlab_http_status(:ok)
+          expect(response.body).not_to include(payload)
+          expect(response.body).to include(html_escape(payload))
+        end
+      end
     end
   end
 end

spec/helpers/integrations_helper_spec.rb

@@ -98,4 +98,19 @@ 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