diff options
-rw-r--r-- | app/assets/javascripts/notebook/cells/markdown.vue | 8 | ||||
-rw-r--r-- | app/assets/javascripts/notebook/cells/output/html.vue | 15 | ||||
-rw-r--r-- | package.json | 1 | ||||
-rw-r--r-- | spec/javascripts/notebook/cells/markdown_spec.js | 12 | ||||
-rw-r--r-- | spec/javascripts/notebook/cells/output/html_sanitize_tests.js | 66 | ||||
-rw-r--r-- | spec/javascripts/notebook/cells/output/html_spec.js | 29 | ||||
-rw-r--r-- | yarn.lock | 49 |
7 files changed, 176 insertions, 4 deletions
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 @@ <script> /* global katex */ import marked from 'marked'; + import sanitize from 'sanitize-html'; import Prompt from './prompt.vue'; const renderer = new marked.Renderer(); @@ -82,7 +83,12 @@ }, computed: { markdown() { - return marked(this.cell.source.join('').replace(/\\/g, '\\\\')); + return sanitize(marked(this.cell.source.join('').replace(/\\/g, '\\\\')), { + allowedTags: false, + allowedAttributes: { + '*': ['class'], + }, + }); }, }, }; diff --git a/app/assets/javascripts/notebook/cells/output/html.vue b/app/assets/javascripts/notebook/cells/output/html.vue index 2110a9de7ed..cb1c7bbe05e 100644 --- a/app/assets/javascripts/notebook/cells/output/html.vue +++ b/app/assets/javascripts/notebook/cells/output/html.vue @@ -1,4 +1,5 @@ <script> +import sanitize from 'sanitize-html'; import Prompt from '../prompt.vue'; export default { @@ -11,12 +12,24 @@ export default { components: { prompt: Prompt, }, + computed: { + sanitizedOutput() { + return sanitize(this.rawCode, { + allowedTags: sanitize.defaults.allowedTags.concat([ + 'img', 'svg', + ]), + allowedAttributes: { + img: ['src'], + }, + }); + }, + }, }; </script> <template> <div class="output"> <prompt /> - <div v-html="rawCode"></div> + <div v-html="sanitizedOutput"></div> </div> </template> diff --git a/package.json b/package.json index 1f83f62e7f7..fb473c292d4 100644 --- a/package.json +++ b/package.json @@ -66,6 +66,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: '<a href="javascript:alert(\'XSS\');">foo</a>', + output: '<a>foo</a>', + }, + 'protocol-based JS injection: simple, spaces before': { + input: '<a href="javascript :alert(\'XSS\');">foo</a>', + output: '<a>foo</a>', + }, + 'protocol-based JS injection: simple, spaces after': { + input: '<a href="javascript: alert(\'XSS\');">foo</a>', + output: '<a>foo</a>', + }, + 'protocol-based JS injection: simple, spaces before and after': { + input: '<a href="javascript : alert(\'XSS\');">foo</a>', + output: '<a>foo</a>', + }, + 'protocol-based JS injection: preceding colon': { + input: '<a href=":javascript:alert(\'XSS\');">foo</a>', + output: '<a>foo</a>', + }, + 'protocol-based JS injection: UTF-8 encoding': { + input: '<a href="javascript:">foo</a>', + output: '<a>foo</a>', + }, + 'protocol-based JS injection: long UTF-8 encoding': { + input: '<a href="javascript:">foo</a>', + output: '<a>foo</a>', + }, + 'protocol-based JS injection: long UTF-8 encoding without semicolons': { + input: '<a href=javascript:alert('XSS')>foo</a>', + output: '<a>foo</a>', + }, + 'protocol-based JS injection: hex encoding': { + input: '<a href="javascript:">foo</a>', + output: '<a>foo</a>', + }, + 'protocol-based JS injection: long hex encoding': { + input: '<a href="javascript:">foo</a>', + output: '<a>foo</a>', + }, + 'protocol-based JS injection: hex encoding without semicolons': { + input: '<a href=javascript:alert('XSS')>foo</a>', + output: '<a>foo</a>', + }, + 'protocol-based JS injection: null char': { + input: '<a href=java\0script:alert("XSS")>foo</a>', + output: '<a>foo</a>', + }, + 'protocol-based JS injection: invalid URL char': { + input: '<img src=java\script:alert("XSS")>', // eslint-disable-line no-useless-escape + output: '<img>', + }, + 'protocol-based JS injection: Unicode': { + input: '<a href="\u0001java\u0003script:alert(\'XSS\')">foo</a>', + output: '<a>foo</a>', + }, + 'protocol-based JS injection: spaces and entities': { + input: '<a href="  javascript:alert(\'XSS\');">foo</a>', + output: '<a>foo</a>', + }, + 'img on error': { + input: '<img src="x" onerror="alert(document.domain)" />', + output: '<img src="x">', + }, +}; 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 da9e50739cd..01786a80524 100644 --- a/yarn.lock +++ b/yarn.lock @@ -233,7 +233,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" @@ -3169,7 +3169,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: @@ -4025,6 +4025,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" @@ -4040,6 +4044,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@^3.7.0: version "3.7.0" resolved "https://registry.yarnpkg.com/lodash.get/-/lodash.get-3.7.0.tgz#3ce68ae2c91683b281cc5394128303cbf75e691f" @@ -4074,6 +4082,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" @@ -5126,6 +5138,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.15" + resolved "https://registry.yarnpkg.com/postcss/-/postcss-6.0.15.tgz#f460cd6269fede0d1bf6defff0b934a9845d974d" + dependencies: + chalk "^2.3.0" + source-map "^0.6.1" + supports-color "^5.1.0" + postcss@^6.0.8: version "6.0.14" resolved "https://registry.yarnpkg.com/postcss/-/postcss-6.0.14.tgz#5534c72114739e75d0afcf017db853099f562885" @@ -5636,6 +5656,18 @@ 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.3" + resolved "https://registry.yarnpkg.com/sanitize-html/-/sanitize-html-1.16.3.tgz#96c1b44a36ff7312e1c22a14b05274370ac8bd56" + dependencies: + htmlparser2 "^3.9.0" + lodash.clonedeep "^4.5.0" + lodash.escaperegexp "^4.1.2" + 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" @@ -5949,6 +5981,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" @@ -6093,6 +6132,12 @@ supports-color@^4.2.1: dependencies: has-flag "^2.0.0" +supports-color@^5.1.0: + version "5.1.0" + resolved "https://registry.yarnpkg.com/supports-color/-/supports-color-5.1.0.tgz#058a021d1b619f7ddf3980d712ea3590ce7de3d5" + 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" |