Escape label and milestone titles to prevent XSS

2 files changed, 51 insertions, 6 deletions

diff --git a/app/assets/javascripts/gfm_auto_complete.js b/app/assets/javascripts/gfm_auto_complete.js
index 337a70a270a..51f05be7c84 100644
--- a/app/assets/javascripts/gfm_auto_complete.js
+++ b/app/assets/javascripts/gfm_auto_complete.js
@@ -242,7 +242,7 @@ class GfmAutoComplete {
       displayTpl(value) {
         let tmpl = GfmAutoComplete.Loading.template;
         if (value.title != null) {
-          tmpl = GfmAutoComplete.Milestones.template;
+          tmpl = GfmAutoComplete.Milestones.templateFunction(value.title);
         }
         return tmpl;
       },
@@ -309,7 +309,7 @@ class GfmAutoComplete {
       searchKey: 'search',
       data: GfmAutoComplete.defaultLoadingData,
       displayTpl(value) {
-        let tmpl = GfmAutoComplete.Labels.template;
+        let tmpl = GfmAutoComplete.Labels.templateFunction(value.color, value.title);
         if (GfmAutoComplete.isLoading(value)) {
           tmpl = GfmAutoComplete.Loading.template;
         }
@@ -559,8 +559,11 @@ GfmAutoComplete.Members = {
   },
 };
 GfmAutoComplete.Labels = {
-  // eslint-disable-next-line no-template-curly-in-string
-  template: '<li><span class="dropdown-label-box" style="background: ${color}"></span> ${title}</li>',
+  templateFunction(color, title) {
+    return `<li><span class="dropdown-label-box" style="background: ${_.escape(
+      color,
+    )}"></span> ${_.escape(title)}</li>`;
+  },
 };
 // Issues, MergeRequests and Snippets
 GfmAutoComplete.Issues = {
@@ -570,8 +573,9 @@ GfmAutoComplete.Issues = {
 };
 // Milestones
 GfmAutoComplete.Milestones = {
-  // eslint-disable-next-line no-template-curly-in-string
-  template: '<li>${title}</li>',
+  templateFunction(title) {
+    return `<li>${_.escape(title)}</li>`;
+  },
 };
 GfmAutoComplete.Loading = {
   template: '<li style="pointer-events: none;"><i class="fa fa-spinner fa-spin"></i> Loading...</li>',
diff --git a/spec/features/issues/gfm_autocomplete_spec.rb b/spec/features/issues/gfm_autocomplete_spec.rb
index 7c591dacce5..b8d7e48510f 100644
--- a/spec/features/issues/gfm_autocomplete_spec.rb
+++ b/spec/features/issues/gfm_autocomplete_spec.rb
@@ -3,6 +3,8 @@ require 'rails_helper'
 describe 'GFM autocomplete', :js do
   let(:issue_xss_title) { 'This will execute alert<img src=x onerror=alert(2)&lt;img src=x onerror=alert(1)&gt;' }
   let(:user_xss_title) { 'eve <img src=x onerror=alert(2)&lt;img src=x onerror=alert(1)&gt;' }
+  let(:label_xss_title) { 'alert label &lt;img src=x onerror="alert(\'Hello xss\');" a'}
+  let(:milestone_xss_title) { 'alert milestone &lt;img src=x onerror="alert(\'Hello xss\');" a' }
 
   let(:user_xss) { create(:user, name: user_xss_title, username: 'xss.user') }
   let(:user)    { create(:user, name: '💃speciąl someone💃', username: 'someone.special') }
@@ -26,10 +28,14 @@ describe 'GFM autocomplete', :js do
 
     simulate_input('#issue-description', "@#{user.name[0...3]}")
 
+    wait_for_requests
+
     find('.atwho-view .cur').click
 
     click_button 'Save changes'
 
+    wait_for_requests
+
     expect(find('.description')).to have_content(user.to_reference)
   end
 
@@ -48,6 +54,8 @@ describe 'GFM autocomplete', :js do
       find('#note-body').native.send_keys('#')
     end
 
+    wait_for_requests
+
     expect(page).to have_selector('.atwho-container')
 
     page.within '.atwho-container #at-view-issues' do
@@ -60,6 +68,8 @@ describe 'GFM autocomplete', :js do
       find('#note-body').native.send_keys('@ev')
     end
 
+    wait_for_requests
+
     expect(page).to have_selector('.atwho-container')
 
     page.within '.atwho-container #at-view-users' do
@@ -67,6 +77,22 @@ describe 'GFM autocomplete', :js do
     end
   end
 
+  it 'opens autocomplete menu for Milestone when field starts with text with item escaping HTML characters' do
+    create(:milestone, project: project, title: milestone_xss_title)
+
+    page.within '.timeline-content-form' do
+      find('#note-body').native.send_keys('%')
+    end
+
+    wait_for_requests
+
+    expect(page).to have_selector('.atwho-container')
+
+    page.within '.atwho-container #at-view-milestones' do
+      expect(find('li').text).to have_content('alert milestone')
+    end
+  end
+
   it 'doesnt open autocomplete menu character is prefixed with text' do
     page.within '.timeline-content-form' do
       find('#note-body').native.send_keys('testing')
@@ -259,6 +285,21 @@ describe 'GFM autocomplete', :js do
     let!(:bug)              { create(:label, project: project, title: 'bug') }
     let!(:feature_proposal) { create(:label, project: project, title: 'feature proposal') }
 
+    it 'opens autocomplete menu for Labels when field starts with text with item escaping HTML characters' do
+      create(:label, project: project, title: label_xss_title)
+
+      note = find('#note-body')
+
+      # It should show all the labels on "~".
+      type(note, '~')
+
+      wait_for_requests
+
+      page.within '.atwho-container #at-view-labels' do
+        expect(find('.atwho-view-ul').text).to have_content('alert label')
+      end
+    end
+
     context 'when no labels are assigned' do
       it 'shows labels' do
         note = find('#note-body')