summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorThiago Presa <tpresa@gitlab.com>2018-10-23 02:21:40 +0000
committerThiago Presa <tpresa@gitlab.com>2018-10-24 21:59:46 -0300
commitbf097697f543bc2551f88895ca08084420e45068 (patch)
treeb5962991b754ce1c18e143a6d696c6f795fa4df7
parent39473cc0addedc3d9b7aebe37b4470904c68eaea (diff)
downloadgitlab-ce-bf097697f543bc2551f88895ca08084420e45068.tar.gz
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.rb12
-rw-r--r--changelogs/unreleased/51527-xss-in-mr-source-branch.yml5
-rw-r--r--spec/presenters/merge_request_presenter_spec.rb9
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