Merge branch 'fl-fix-milestone-bug-10-6' into 'security-10-6'

Escape miletone attribute when appending to the DOM

See merge request gitlab/gitlabhq!2359

2 files changed, 21 insertions, 4 deletions

diff --git a/app/assets/javascripts/milestone_select.js b/app/assets/javascripts/milestone_select.js
index add07c156a4..c749042a14a 100644
--- a/app/assets/javascripts/milestone_select.js
+++ b/app/assets/javascripts/milestone_select.js
@@ -94,10 +94,10 @@ export default class MilestoneSelect {
             if (showMenuAbove) {
               $dropdown.data('glDropdown').positionMenuAbove();
             }
-            $(`[data-milestone-id="${selectedMilestone}"] > a`).addClass('is-active');
+            $(`[data-milestone-id="${_.escape(selectedMilestone)}"] > a`).addClass('is-active');
           }),
         renderRow: milestone => `
-          <li data-milestone-id="${milestone.name}">
+          <li data-milestone-id="${_.escape(milestone.name)}">
             <a href='#' class='dropdown-menu-milestone-link'>
               ${_.escape(milestone.title)}
             </a>
@@ -125,7 +125,6 @@ export default class MilestoneSelect {
             return milestone.id;
           }
         },
-        isSelected: milestone => milestone.name === selectedMilestone,
         hidden: () => {
           $selectBox.hide();
           // display:block overrides the hide-collapse rule
@@ -137,7 +136,7 @@ export default class MilestoneSelect {
             selectedMilestone = $dropdown[0].dataset.selected || selectedMilestoneDefault;
           }
           $('a.is-active', $el).removeClass('is-active');
-          $(`[data-milestone-id="${selectedMilestone}"] > a`, $el).addClass('is-active');
+          $(`[data-milestone-id="${_.escape(selectedMilestone)}"] > a`, $el).addClass('is-active');
         },
         vue: $dropdown.hasClass('js-issue-board-sidebar'),
         clicked: (clickEvent) => {
@@ -158,6 +157,7 @@ export default class MilestoneSelect {
           const isMRIndex = (page === page && page === 'projects:merge_requests:index');
           const isSelecting = (selected.name !== selectedMilestone);
           selectedMilestone = isSelecting ? selected.name : selectedMilestoneDefault;
+
           if ($dropdown.hasClass('js-filter-bulk-update') || $dropdown.hasClass('js-issuable-form-dropdown')) {
             e.preventDefault();
             return;
diff --git a/spec/features/issues/form_spec.rb b/spec/features/issues/form_spec.rb
index 38c618d300e..4625a50b8d9 100644
--- a/spec/features/issues/form_spec.rb
+++ b/spec/features/issues/form_spec.rb
@@ -226,6 +226,23 @@ describe 'New/edit issue', :js do
 
       expect(page).to have_selector('.atwho-view')
     end
+
+    describe 'milestone' do
+      let!(:milestone) { create(:milestone, title: '">&lt;img src=x onerror=alert(document.domain)&gt;', project: project) }
+
+      it 'escapes milestone' do
+        click_button 'Milestone'
+
+        page.within '.issue-milestone' do
+          click_link milestone.title
+        end
+
+        page.within '.js-milestone-select' do
+          expect(page).to have_content milestone.title
+          expect(page).not_to have_selector 'img'
+        end
+      end
+    end
   end
 
   context 'edit issue' do