Commit 8c4450ee authored by Tom Quirk's avatar Tom Quirk Committed by Markus Koller

Escape issue reference and title for Jira issues

Changelog: security
EE: true
parent 6f9481a6
...@@ -137,7 +137,7 @@ module IntegrationsHelper ...@@ -137,7 +137,7 @@ module IntegrationsHelper
def jira_issue_breadcrumb_link(issue_reference) def jira_issue_breadcrumb_link(issue_reference)
link_to '', { class: 'gl-display-flex gl-align-items-center gl-white-space-nowrap' } do 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 = 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
end end
......
- add_to_breadcrumbs _('Jira Issues'), project_integrations_jira_issues_path(@project) - add_to_breadcrumbs _('Jira Issues'), project_integrations_jira_issues_path(@project)
- breadcrumb_title jira_issue_breadcrumb_link(@issue_json[:references][:relative]) - 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 } .js-jira-issues-show-app{ data: jira_issues_show_data }
...@@ -203,12 +203,12 @@ RSpec.describe Projects::Integrations::Jira::IssuesController do ...@@ -203,12 +203,12 @@ RSpec.describe Projects::Integrations::Jira::IssuesController do
before do before do
stub_licensed_features(jira_issues_integration: true) stub_licensed_features(jira_issues_integration: true)
expect_next_found_instance_of(Integrations::Jira) do |service| allow_next_found_instance_of(Integrations::Jira) do |service|
expect(service).to receive(:find_issue).with('1', rendered_fields: true).and_return(jira_issue) allow(service).to receive(:find_issue).with('1', rendered_fields: true).and_return(jira_issue)
end end
expect_next_instance_of(Integrations::JiraSerializers::IssueDetailSerializer) do |serializer| allow_next_instance_of(Integrations::JiraSerializers::IssueDetailSerializer) do |serializer|
expect(serializer).to receive(:represent).with(jira_issue, project: project).and_return(issue_json) allow(serializer).to receive(:represent).with(jira_issue, project: project).and_return(issue_json)
end end
end end
...@@ -225,6 +225,21 @@ RSpec.describe Projects::Integrations::Jira::IssuesController do ...@@ -225,6 +225,21 @@ RSpec.describe Projects::Integrations::Jira::IssuesController do
expect(json_response).to eq(issue_json) expect(json_response).to eq(issue_json)
end 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 end
end end
...@@ -98,4 +98,19 @@ RSpec.describe IntegrationsHelper do ...@@ -98,4 +98,19 @@ RSpec.describe IntegrationsHelper do
end end
end 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 end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment