Merge branch 'security-html_escape_branch_name-10-8' into 'security-10-8'

[10.8] HTML escape branch name in project graphs page

See merge request gitlab/gitlabhq!2414

3 files changed, 25 insertions, 2 deletions

diff --git a/app/views/projects/graphs/charts.html.haml b/app/views/projects/graphs/charts.html.haml
index 14c47a5d91c..9e6fd49c263 100644
--- a/app/views/projects/graphs/charts.html.haml
+++ b/app/views/projects/graphs/charts.html.haml
@@ -30,7 +30,7 @@
           #{@commits_graph.start_date.strftime('%b %d')}
         - end_time = capture do
           #{@commits_graph.end_date.strftime('%b %d')}
-        = (_("Commit statistics for %{ref} %{start_time} - %{end_time}") % { ref: "<strong>#{@ref}</strong>", start_time: start_time, end_time: end_time }).html_safe
+        = (_("Commit statistics for %{ref} %{start_time} - %{end_time}") % { ref: "<strong>#{h @ref}</strong>", start_time: start_time, end_time: end_time }).html_safe
 
     .col-md-6
       .tree-ref-container
diff --git a/changelogs/unreleased/security-html_escape_branch_name.yml b/changelogs/unreleased/security-html_escape_branch_name.yml
new file mode 100644
index 00000000000..02d1065348f
--- /dev/null
+++ b/changelogs/unreleased/security-html_escape_branch_name.yml
@@ -0,0 +1,5 @@
+---
+title: HTML escape branch name in project graphs page
+merge_request:
+author:
+type: security
diff --git a/spec/features/projects/graph_spec.rb b/spec/features/projects/graph_spec.rb
index 57172610aed..335174b7729 100644
--- a/spec/features/projects/graph_spec.rb
+++ b/spec/features/projects/graph_spec.rb
@@ -3,6 +3,7 @@ require 'spec_helper'
 describe 'Project Graph', :js do
   let(:user) { create :user }
   let(:project) { create(:project, :repository, namespace: user.namespace) }
+  let(:branch_name) { 'master' }
 
   before do
     project.add_master(user)
@@ -12,7 +13,7 @@ describe 'Project Graph', :js do
 
   shared_examples 'page should have commits graphs' do
     it 'renders commits' do
-      expect(page).to have_content('Commit statistics for master')
+      expect(page).to have_content("Commit statistics for #{branch_name}")
       expect(page).to have_content('Commits per day of month')
     end
   end
@@ -57,6 +58,23 @@ describe 'Project Graph', :js do
     it_behaves_like 'page should have languages graphs'
   end
 
+  context 'chart graph with HTML escaped branch name' do
+    let(:branch_name) { '<h1>evil</h1>' }
+
+    before do
+      project.repository.create_branch(branch_name, 'master')
+
+      visit charts_project_graph_path(project, branch_name)
+    end
+
+    it_behaves_like 'page should have commits graphs'
+
+    it 'HTML escapes branch name' do
+      expect(page.body).to include("Commit statistics for <strong>#{ERB::Util.html_escape(branch_name)}</strong>")
+      expect(page.body).not_to include(branch_name)
+    end
+  end
+
   context 'when CI enabled' do
     before do
       project.enable_ci