From 8043a3112c1a22d68193e41c85a501a74caeab21 Mon Sep 17 00:00:00 2001 From: Jacob Schatz Date: Thu, 14 Dec 2017 20:07:45 +0000 Subject: Merge branch 'ipython-xss-fix' into 'security-10-2' Sanitizes IPython notebook output See merge request gitlab/gitlabhq!2237 (cherry picked from commit db98d764c4112dd24bc5ae9ed2bfc01052820309) 8908edbf Sanitizes iPython notebook output 90286ceb fixed karma specs --- app/assets/javascripts/notebook/cells/markdown.vue | 8 ++- .../javascripts/notebook/cells/output/html.vue | 15 ++++- changelogs/unreleased/ipython-xss-fix.yml | 5 ++ package.json | 1 + spec/javascripts/notebook/cells/markdown_spec.js | 12 ++++ .../notebook/cells/output/html_sanitize_tests.js | 66 ++++++++++++++++++++++ .../javascripts/notebook/cells/output/html_spec.js | 29 ++++++++++ yarn.lock | 58 ++++++++++++++++++- 8 files changed, 190 insertions(+), 4 deletions(-) create mode 100644 changelogs/unreleased/ipython-xss-fix.yml create mode 100644 spec/javascripts/notebook/cells/output/html_sanitize_tests.js create mode 100644 spec/javascripts/notebook/cells/output/html_spec.js diff --git a/app/assets/javascripts/notebook/cells/markdown.vue b/app/assets/javascripts/notebook/cells/markdown.vue index 82c51a1068c..721753af595 100644 --- a/app/assets/javascripts/notebook/cells/markdown.vue +++ b/app/assets/javascripts/notebook/cells/markdown.vue @@ -1,6 +1,7 @@ diff --git a/changelogs/unreleased/ipython-xss-fix.yml b/changelogs/unreleased/ipython-xss-fix.yml new file mode 100644 index 00000000000..619287992d6 --- /dev/null +++ b/changelogs/unreleased/ipython-xss-fix.yml @@ -0,0 +1,5 @@ +--- +title: Fixed IPython notebook output not being sanitized +merge_request: +author: +type: security diff --git a/package.json b/package.json index 6315f9eced9..301f1de5a3c 100644 --- a/package.json +++ b/package.json @@ -54,6 +54,7 @@ "raven-js": "^3.14.0", "raw-loader": "^0.5.1", "react-dev-utils": "^0.5.2", + "sanitize-html": "^1.16.1", "select2": "3.5.2-browserify", "sql.js": "^0.4.0", "svg4everybody": "2.1.9", diff --git a/spec/javascripts/notebook/cells/markdown_spec.js b/spec/javascripts/notebook/cells/markdown_spec.js index a88e9ed3d99..db2a16b0b68 100644 --- a/spec/javascripts/notebook/cells/markdown_spec.js +++ b/spec/javascripts/notebook/cells/markdown_spec.js @@ -42,6 +42,18 @@ describe('Markdown component', () => { expect(vm.$el.querySelector('.markdown h1')).not.toBeNull(); }); + it('sanitizes output', (done) => { + Object.assign(cell, { + source: ['[XSS](data:text/html;base64,PHNjcmlwdD5hbGVydChkb2N1bWVudC5kb21haW4pPC9zY3JpcHQ+Cg==)\n'], + }); + + Vue.nextTick(() => { + expect(vm.$el.querySelector('a').getAttribute('href')).toBeNull(); + + done(); + }); + }); + describe('katex', () => { beforeEach(() => { json = getJSONFixture('blob/notebook/math.json'); diff --git a/spec/javascripts/notebook/cells/output/html_sanitize_tests.js b/spec/javascripts/notebook/cells/output/html_sanitize_tests.js new file mode 100644 index 00000000000..d587573fc9e --- /dev/null +++ b/spec/javascripts/notebook/cells/output/html_sanitize_tests.js @@ -0,0 +1,66 @@ +export default { + 'protocol-based JS injection: simple, no spaces': { + input: 'foo', + output: 'foo', + }, + 'protocol-based JS injection: simple, spaces before': { + input: 'foo', + output: 'foo', + }, + 'protocol-based JS injection: simple, spaces after': { + input: 'foo', + output: 'foo', + }, + 'protocol-based JS injection: simple, spaces before and after': { + input: 'foo', + output: 'foo', + }, + 'protocol-based JS injection: preceding colon': { + input: 'foo', + output: 'foo', + }, + 'protocol-based JS injection: UTF-8 encoding': { + input: 'foo', + output: 'foo', + }, + 'protocol-based JS injection: long UTF-8 encoding': { + input: 'foo', + output: 'foo', + }, + 'protocol-based JS injection: long UTF-8 encoding without semicolons': { + input: 'foo', + output: 'foo', + }, + 'protocol-based JS injection: hex encoding': { + input: 'foo', + output: 'foo', + }, + 'protocol-based JS injection: long hex encoding': { + input: 'foo', + output: 'foo', + }, + 'protocol-based JS injection: hex encoding without semicolons': { + input: 'foo', + output: 'foo', + }, + 'protocol-based JS injection: null char': { + input: 'foo', + output: 'foo', + }, + 'protocol-based JS injection: invalid URL char': { + input: '', // eslint-disable-line no-useless-escape + output: '', + }, + 'protocol-based JS injection: Unicode': { + input: 'foo', + output: 'foo', + }, + 'protocol-based JS injection: spaces and entities': { + input: 'foo', + output: 'foo', + }, + 'img on error': { + input: '', + output: '', + }, +}; diff --git a/spec/javascripts/notebook/cells/output/html_spec.js b/spec/javascripts/notebook/cells/output/html_spec.js new file mode 100644 index 00000000000..9c5385f2922 --- /dev/null +++ b/spec/javascripts/notebook/cells/output/html_spec.js @@ -0,0 +1,29 @@ +import Vue from 'vue'; +import htmlOutput from '~/notebook/cells/output/html.vue'; +import sanitizeTests from './html_sanitize_tests'; + +describe('html output cell', () => { + function createComponent(rawCode) { + const Component = Vue.extend(htmlOutput); + + return new Component({ + propsData: { + rawCode, + }, + }).$mount(); + } + + describe('sanitizes output', () => { + Object.keys(sanitizeTests).forEach((key) => { + it(key, () => { + const test = sanitizeTests[key]; + const vm = createComponent(test.input); + const outputEl = [...vm.$el.querySelectorAll('div')].pop(); + + expect(outputEl.innerHTML).toEqual(test.output); + + vm.$destroy(); + }); + }); + }); +}); diff --git a/yarn.lock b/yarn.lock index efae3892c31..71d43755a55 100644 --- a/yarn.lock +++ b/yarn.lock @@ -173,7 +173,7 @@ array-union@^1.0.1: dependencies: array-uniq "^1.0.1" -array-uniq@^1.0.1: +array-uniq@^1.0.1, array-uniq@^1.0.2: version "1.0.3" resolved "https://registry.yarnpkg.com/array-uniq/-/array-uniq-1.0.3.tgz#af6ac877a25cc7f74e058894753858dfdb24fdb6" @@ -2962,7 +2962,7 @@ html-entities@1.2.0, html-entities@^1.2.0: version "1.2.0" resolved "https://registry.yarnpkg.com/html-entities/-/html-entities-1.2.0.tgz#41948caf85ce82fed36e4e6a0ed371a6664379e2" -htmlparser2@^3.8.2: +htmlparser2@^3.8.2, htmlparser2@^3.9.0: version "3.9.2" resolved "https://registry.yarnpkg.com/htmlparser2/-/htmlparser2-3.9.2.tgz#1bdf87acca0f3f9e53fa4fcceb0f4b4cbb00b338" dependencies: @@ -3827,6 +3827,10 @@ lodash.capitalize@^4.0.0: version "4.2.1" resolved "https://registry.yarnpkg.com/lodash.capitalize/-/lodash.capitalize-4.2.1.tgz#f826c9b4e2a8511d84e3aca29db05e1a4f3b72a9" +lodash.clonedeep@^4.5.0: + version "4.5.0" + resolved "https://registry.yarnpkg.com/lodash.clonedeep/-/lodash.clonedeep-4.5.0.tgz#e23f3f9c4f8fbdde872529c1071857a086e5ccef" + lodash.cond@^4.3.0: version "4.5.2" resolved "https://registry.yarnpkg.com/lodash.cond/-/lodash.cond-4.5.2.tgz#f471a1da486be60f6ab955d17115523dd1d255d5" @@ -3842,6 +3846,10 @@ lodash.defaults@^3.1.2: lodash.assign "^3.0.0" lodash.restparam "^3.0.0" +lodash.escaperegexp@^4.1.2: + version "4.1.2" + resolved "https://registry.yarnpkg.com/lodash.escaperegexp/-/lodash.escaperegexp-4.1.2.tgz#64762c48618082518ac3df4ccf5d5886dae20347" + lodash.get@4.4.2: version "4.4.2" resolved "https://registry.yarnpkg.com/lodash.get/-/lodash.get-4.4.2.tgz#2d177f652fa31e939b4438d5341499dfa3825e99" @@ -3861,6 +3869,10 @@ lodash.isarray@^3.0.0: version "3.0.4" resolved "https://registry.yarnpkg.com/lodash.isarray/-/lodash.isarray-3.0.4.tgz#79e4eb88c36a8122af86f844aa9bcd851b5fbb55" +lodash.isarray@^4.0.0: + version "4.0.0" + resolved "https://registry.yarnpkg.com/lodash.isarray/-/lodash.isarray-4.0.0.tgz#2aca496b28c4ca6d726715313590c02e6ea34403" + lodash.kebabcase@4.0.1: version "4.0.1" resolved "https://registry.yarnpkg.com/lodash.kebabcase/-/lodash.kebabcase-4.0.1.tgz#5e63bc9aa2a5562ff3b97ca7af2f803de1bcb90e" @@ -3880,6 +3892,10 @@ lodash.memoize@^4.1.2: version "4.1.2" resolved "https://registry.yarnpkg.com/lodash.memoize/-/lodash.memoize-4.1.2.tgz#bcc6c49a42a2840ed997f323eada5ecd182e0bfe" +lodash.mergewith@^4.6.0: + version "4.6.0" + resolved "https://registry.yarnpkg.com/lodash.mergewith/-/lodash.mergewith-4.6.0.tgz#150cf0a16791f5903b8891eab154609274bdea55" + lodash.restparam@^3.0.0: version "3.6.1" resolved "https://registry.yarnpkg.com/lodash.restparam/-/lodash.restparam-3.6.1.tgz#936a4e309ef330a7645ed4145986c85ae5b20805" @@ -4938,6 +4954,14 @@ postcss@^5.0.10, postcss@^5.0.11, postcss@^5.0.12, postcss@^5.0.13, postcss@^5.0 source-map "^0.5.6" supports-color "^3.2.3" +postcss@^6.0.14: + version "6.0.14" + resolved "https://registry.yarnpkg.com/postcss/-/postcss-6.0.14.tgz#5534c72114739e75d0afcf017db853099f562885" + dependencies: + chalk "^2.3.0" + source-map "^0.6.1" + supports-color "^4.4.0" + prelude-ls@~1.1.2: version "1.1.2" resolved "https://registry.yarnpkg.com/prelude-ls/-/prelude-ls-1.1.2.tgz#21932a549f5e52ffd9a827f570e04be62a97da54" @@ -5434,6 +5458,19 @@ safe-buffer@^5.0.1, safe-buffer@~5.0.1: version "5.0.1" resolved "https://registry.yarnpkg.com/safe-buffer/-/safe-buffer-5.0.1.tgz#d263ca54696cd8a306b5ca6551e92de57918fbe7" +sanitize-html@^1.16.1: + version "1.16.1" + resolved "https://registry.yarnpkg.com/sanitize-html/-/sanitize-html-1.16.1.tgz#415e9b7f2d59fff2df9e4eec4067046e63e93b9c" + dependencies: + htmlparser2 "^3.9.0" + lodash.clonedeep "^4.5.0" + lodash.escaperegexp "^4.1.2" + lodash.isarray "^4.0.0" + lodash.mergewith "^4.6.0" + postcss "^6.0.14" + srcset "^1.0.0" + xtend "^4.0.0" + sax@~1.2.1: version "1.2.2" resolved "https://registry.yarnpkg.com/sax/-/sax-1.2.2.tgz#fd8631a23bc7826bef5d871bdb87378c95647828" @@ -5684,6 +5721,10 @@ source-map@^0.4.4: dependencies: amdefine ">=0.0.4" +source-map@^0.6.1: + version "0.6.1" + resolved "https://registry.yarnpkg.com/source-map/-/source-map-0.6.1.tgz#74722af32e9614e9c287a8d0bbde48b5e2f1a263" + source-map@~0.2.0: version "0.2.0" resolved "https://registry.yarnpkg.com/source-map/-/source-map-0.2.0.tgz#dab73fbcfc2ba819b4de03bd6f6eaa48164b3f9d" @@ -5741,6 +5782,13 @@ sql.js@^0.4.0: version "0.4.0" resolved "https://registry.yarnpkg.com/sql.js/-/sql.js-0.4.0.tgz#23be9635520eb0ff43a741e7e830397266e88445" +srcset@^1.0.0: + version "1.0.0" + resolved "https://registry.yarnpkg.com/srcset/-/srcset-1.0.0.tgz#a5669de12b42f3b1d5e83ed03c71046fc48f41ef" + dependencies: + array-uniq "^1.0.2" + number-is-nan "^1.0.0" + sshpk@^1.7.0: version "1.13.1" resolved "https://registry.yarnpkg.com/sshpk/-/sshpk-1.13.1.tgz#512df6da6287144316dc4c18fe1cf1d940739be3" @@ -5879,6 +5927,12 @@ supports-color@^4.0.0, supports-color@^4.2.1: dependencies: has-flag "^2.0.0" +supports-color@^4.4.0: + version "4.5.0" + resolved "https://registry.yarnpkg.com/supports-color/-/supports-color-4.5.0.tgz#be7a0de484dec5c5cddf8b3d59125044912f635b" + dependencies: + has-flag "^2.0.0" + svg4everybody@2.1.9: version "2.1.9" resolved "https://registry.yarnpkg.com/svg4everybody/-/svg4everybody-2.1.9.tgz#5bd9f6defc133859a044646d4743fabc28db7e2d" -- cgit v1.2.1