Merge branch 'security-acet-issue-details-11-2' into 'security-11-2'

[11.2] Fix XSS on Issue details page.

See merge request gitlab/gitlabhq!2471

5 files changed, 47 insertions, 4 deletions

diff --git a/app/assets/javascripts/issue_show/index.js b/app/assets/javascripts/issue_show/index.js
index 75dfdedcf1b..d08e8ba0c4b 100644
--- a/app/assets/javascripts/issue_show/index.js
+++ b/app/assets/javascripts/issue_show/index.js
@@ -1,10 +1,11 @@
 import Vue from 'vue';
+import sanitize from 'sanitize-html';
 import issuableApp from './components/app.vue';
 import '../vue_shared/vue_resource_interceptor';
 
-document.addEventListener('DOMContentLoaded', () => {
+export default function initIssueableApp() {
   const initialDataEl = document.getElementById('js-issuable-app-initial-data');
-  const props = JSON.parse(initialDataEl.innerHTML.replace(/"/g, '"'));
+  const props = JSON.parse(sanitize(initialDataEl.textContent).replace(/"/g, '"'));
 
   return new Vue({
     el: document.getElementById('js-issuable-app'),
@@ -17,4 +18,4 @@ document.addEventListener('DOMContentLoaded', () => {
       });
     },
   });
-});
+}
diff --git a/app/assets/javascripts/pages/projects/issues/show.js b/app/assets/javascripts/pages/projects/issues/show.js
index 500fbd27340..3f415391aa1 100644
--- a/app/assets/javascripts/pages/projects/issues/show.js
+++ b/app/assets/javascripts/pages/projects/issues/show.js
@@ -3,9 +3,10 @@ import Issue from '~/issue';
 import ShortcutsIssuable from '~/shortcuts_issuable';
 import ZenMode from '~/zen_mode';
 import '~/notes/index';
-import '~/issue_show/index';
+import initIssueableApp from '~/issue_show';
 
 export default function () {
+  initIssueableApp();
   new Issue(); // eslint-disable-line no-new
   new ShortcutsIssuable(); // eslint-disable-line no-new
   new ZenMode(); // eslint-disable-line no-new
diff --git a/changelogs/unreleased/security-acet-issue-details.yml b/changelogs/unreleased/security-acet-issue-details.yml
new file mode 100644
index 00000000000..64147a9d6e8
--- /dev/null
+++ b/changelogs/unreleased/security-acet-issue-details.yml
@@ -0,0 +1,5 @@
+---
+title: Sanitize JSON data properly to fix XSS on Issue details page
+merge_request:
+author:
+type: security
diff --git a/spec/features/issues/issue_detail_spec.rb b/spec/features/issues/issue_detail_spec.rb
index 088ab114df3..76bc93e9766 100644
--- a/spec/features/issues/issue_detail_spec.rb
+++ b/spec/features/issues/issue_detail_spec.rb
@@ -18,6 +18,23 @@ describe 'Issue Detail', :js do
     end
   end
 
+  context 'when issue description has xss snippet' do
+    before do
+      issue.update!(description: '![xss" onload=alert(1);//](a)')
+      sign_in(user)
+      visit project_issue_path(project, issue)
+      wait_for_requests
+    end
+
+    it 'should encode the description to prevent xss issues' do
+      page.within('.issuable-details .detail-page-description') do
+        expect(page).to have_selector('img', count: 1)
+        expect(find('img')['onerror']).to be_nil
+        expect(find('img')['src']).to end_with('/a')
+      end
+    end
+  end
+
   context 'when edited by a user who is later deleted' do
     before do
       sign_in(user)
diff --git a/spec/javascripts/issue_show/index_spec.js b/spec/javascripts/issue_show/index_spec.js
new file mode 100644
index 00000000000..fa0b426c06c
--- /dev/null
+++ b/spec/javascripts/issue_show/index_spec.js
@@ -0,0 +1,19 @@
+import initIssueableApp from '~/issue_show';
+
+describe('Issue show index', () => {
+  describe('initIssueableApp', () => {
+    it('should initialize app with no potential XSS attack', () => {
+      const d = document.createElement('div');
+      d.id = 'js-issuable-app-initial-data';
+      d.innerHTML = JSON.stringify({
+        initialDescriptionHtml: '<img src=x onerror=alert(1)>',
+      });
+      document.body.appendChild(d);
+
+      const alertSpy = spyOn(window, 'alert');
+      initIssueableApp();
+
+      expect(alertSpy).not.toHaveBeenCalled();
+    });
+  });
+});