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

8 files changed, 190 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/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: '<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&#58;">foo</a>',
+    output: '<a>foo</a>',
+  },
+  'protocol-based JS injection: long UTF-8 encoding': {
+    input: '<a href="javascript&#0058;">foo</a>',
+    output: '<a>foo</a>',
+  },
+  'protocol-based JS injection: long UTF-8 encoding without semicolons': {
+    input: '<a href=&#0000106&#0000097&#0000118&#0000097&#0000115&#0000099&#0000114&#0000105&#0000112&#0000116&#0000058&#0000097&#0000108&#0000101&#0000114&#0000116&#0000040&#0000039&#0000088&#0000083&#0000083&#0000039&#0000041>foo</a>',
+    output: '<a>foo</a>',
+  },
+  'protocol-based JS injection: hex encoding': {
+    input: '<a href="javascript&#x3A;">foo</a>',
+    output: '<a>foo</a>',
+  },
+  'protocol-based JS injection: long hex encoding': {
+    input: '<a href="javascript&#x003A;">foo</a>',
+    output: '<a>foo</a>',
+  },
+  'protocol-based JS injection: hex encoding without semicolons': {
+    input: '<a href=&#x6A&#x61&#x76&#x61&#x73&#x63&#x72&#x69&#x70&#x74&#x3A&#x61&#x6C&#x65&#x72&#x74&#x28&#x27&#x58&#x53&#x53&#x27&#x29>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=" &#14;  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 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"