summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--app/assets/javascripts/issue_show/index.js7
-rw-r--r--app/assets/javascripts/pages/projects/issues/show.js3
-rw-r--r--changelogs/unreleased/security-acet-issue-details.yml5
-rw-r--r--spec/features/issues/issue_detail_spec.rb17
-rw-r--r--spec/javascripts/issue_show/index_spec.js19
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();
+ });
+ });
+});