diff options
author | Thiago Presa <tpresa@gitlab.com> | 2018-10-23 05:21:40 +0300 |
---|---|---|
committer | Thiago Presa <tpresa@gitlab.com> | 2018-10-25 03:59:46 +0300 |
commit | bf097697f543bc2551f88895ca08084420e45068 (patch) | |
tree | b5962991b754ce1c18e143a6d696c6f795fa4df7 | |
parent | 39473cc0addedc3d9b7aebe37b4470904c68eaea (diff) |
Merge branch 'security-11-2-51527-xss-in-mr-source-branch' into 'security-11-2'
[11.2] Fix XSS in MR source branch name
See merge request gitlab/gitlabhq!2546
-rw-r--r-- | app/presenters/merge_request_presenter.rb | 12 | ||||
-rw-r--r-- | changelogs/unreleased/51527-xss-in-mr-source-branch.yml | 5 | ||||
-rw-r--r-- | spec/presenters/merge_request_presenter_spec.rb | 9 |
3 files changed, 17 insertions, 9 deletions
diff --git a/app/presenters/merge_request_presenter.rb b/app/presenters/merge_request_presenter.rb index 8c4eac3c31d..800d0196729 100644 --- a/app/presenters/merge_request_presenter.rb +++ b/app/presenters/merge_request_presenter.rb @@ -108,16 +108,10 @@ class MergeRequestPresenter < Gitlab::View::Presenter::Delegated namespace = source_project_namespace branch = source_branch - if source_branch_exists? - namespace = link_to(namespace, project_path(source_project)) - branch = link_to(branch, project_tree_path(source_project, source_branch)) - end + namespace_link = source_branch_exists? ? link_to(namespace, project_path(source_project)) : ERB::Util.html_escape(namespace) + branch_link = source_branch_exists? ? link_to(branch, project_tree_path(source_project, source_branch)) : ERB::Util.html_escape(branch) - if for_fork? - namespace + ":" + branch - else - branch - end + for_fork? ? "#{namespace_link}:#{branch_link}" : branch_link end def closing_issues_links diff --git a/changelogs/unreleased/51527-xss-in-mr-source-branch.yml b/changelogs/unreleased/51527-xss-in-mr-source-branch.yml new file mode 100644 index 00000000000..dae277b6413 --- /dev/null +++ b/changelogs/unreleased/51527-xss-in-mr-source-branch.yml @@ -0,0 +1,5 @@ +--- +title: Fix XSS in merge request source branch name +merge_request: +author: +type: security diff --git a/spec/presenters/merge_request_presenter_spec.rb b/spec/presenters/merge_request_presenter_spec.rb index a1b52d8692d..bafcddebbb7 100644 --- a/spec/presenters/merge_request_presenter_spec.rb +++ b/spec/presenters/merge_request_presenter_spec.rb @@ -403,6 +403,15 @@ describe MergeRequestPresenter do is_expected .to eq("<a href=\"/#{resource.source_project.full_path}/tree/#{resource.source_branch}\">#{resource.source_branch}</a>") end + + it 'escapes html, when source_branch does not exist' do + xss_attempt = "<img src='x' onerror=alert('bad stuff') />" + + allow(resource).to receive(:source_branch) { xss_attempt } + allow(resource).to receive(:source_branch_exists?) { false } + + is_expected.to eq(ERB::Util.html_escape(xss_attempt)) + end end describe '#rebase_path' do |