From 8c55d0d8790587f2b73a912c4385b9ee7e2c32b7 Mon Sep 17 00:00:00 2001 From: Julien Ramboz Date: Mon, 15 Apr 2019 15:53:28 -0700 Subject: [PATCH] fix(html pipe): Sanitize generated markdown to avoid XSS attacks Properly escape the initial markdown and custom matchers to avoid potential XSS attacks via JS injection, and also avoid DOM clubbering fix #253 --- package-lock.json | 8 +++++++ src/utils/mdast-to-vdom.js | 1 + src/utils/sanitize-hast.js | 46 ++++++++++++++++++++++++++++++++++++ test/testHTMLFromMarkdown.js | 24 +++++++++++++++++++ test/testMdastToVDOM.js | 6 ++--- 5 files changed, 82 insertions(+), 3 deletions(-) create mode 100644 src/utils/sanitize-hast.js diff --git a/package-lock.json b/package-lock.json index 5a44f3c47..fa910c119 100644 --- a/package-lock.json +++ b/package-lock.json @@ -5361,6 +5361,14 @@ "zwitch": "^1.0.0" } }, + "hast-util-sanitize": { + "version": "1.3.0", + "resolved": "https://registry.npmjs.org/hast-util-sanitize/-/hast-util-sanitize-1.3.0.tgz", + "integrity": "sha512-rQeetoD08jHmDOUYN6h9vTuE0hQN4wymhtkQZ6whHtcjaLpjw5RYAbcdxx9cMgMWERDsSs79UpqHuBLlUHKeOw==", + "requires": { + "xtend": "^4.0.1" + } + }, "hast-util-to-html": { "version": "5.0.1", "resolved": "https://registry.npmjs.org/hast-util-to-html/-/hast-util-to-html-5.0.1.tgz", diff --git a/src/utils/mdast-to-vdom.js b/src/utils/mdast-to-vdom.js index ef68f76ac..1768f2516 100644 --- a/src/utils/mdast-to-vdom.js +++ b/src/utils/mdast-to-vdom.js @@ -19,6 +19,7 @@ const unified = require('unified'); const parse = require('rehype-parse'); const { JSDOM } = require('jsdom'); const HeadingHandler = require('./heading-handler'); +const sanitize = require('./sanitize-hast'); const image = require('./image-handler'); const embed = require('./embed-handler'); const link = require('./link-handler'); diff --git a/src/utils/sanitize-hast.js b/src/utils/sanitize-hast.js new file mode 100644 index 000000000..96ad0cede --- /dev/null +++ b/src/utils/sanitize-hast.js @@ -0,0 +1,46 @@ +/* + * Copyright 2019 Adobe. All rights reserved. + * This file is licensed to you under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. You may obtain a copy + * of the License at http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under + * the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS + * OF ANY KIND, either express or implied. See the License for the specific language + * governing permissions and limitations under the License. + */ +const _ = require('lodash'); +const sanitize = require('hast-util-sanitize'); +const GITHUB_SCHEMA = require('hast-util-sanitize/lib/github'); + +// Define helix specific customizations on top of the default GitHub schema +const HELIX_SCHEMA_CUSTOMIZATIONS = { + tagNames: ['esi:include'], + attributes: { + 'esi:include': ['src'], + code: ['className'], + img: ['sizes', 'srcset'], + }, +}; + +/** + * A customizer function for lodash's mergeWith method that concats arrays. + * @param {*} obj The object to merge into + * @param {*} src The object to merge from + * @returns {Array} the concatenanted array or undefined if the object is not an array + */ +function arrayMergeCustomizer(obj, src) { + if (_.isArray(obj)) { + return obj.concat(src); + } + return undefined; +} + +// Define the sanitization schema for helix +const HELIX_SCHEMA = _.mergeWith( + GITHUB_SCHEMA, + HELIX_SCHEMA_CUSTOMIZATIONS, + arrayMergeCustomizer, +); + +module.exports = hast => sanitize(hast, HELIX_SCHEMA); diff --git a/test/testHTMLFromMarkdown.js b/test/testHTMLFromMarkdown.js index bf4f54033..ddc9ccd49 100644 --- a/test/testHTMLFromMarkdown.js +++ b/test/testHTMLFromMarkdown.js @@ -266,4 +266,28 @@ describe('Testing Markdown conversion', () => { `); }); + + it('XSS escape href attribute on links', async () => { + await assertMd(` + [Foo](javascript://%0Dalert('XSS!')) + [Bar](javascript:alert('XSS!')) + `, ` +

+ Foo + Bar +

+ `); + }); + + it('XSS escape href in images', async () => { + await assertMd(` + ![Foo](javascript://%0Dalert('XSS!')) + ![Bar](javascript:alert('XSS!')) + `, ` +

+ Foo + Bar +

+ `); + }); }); diff --git a/test/testMdastToVDOM.js b/test/testMdastToVDOM.js index 83d415299..5004d3971 100644 --- a/test/testMdastToVDOM.js +++ b/test/testMdastToVDOM.js @@ -94,10 +94,10 @@ describe('Test MDAST to VDOM Transformation', () => { it('Programmatic Matcher Function', () => { const mdast = fs.readJSONSync(path.resolve(__dirname, 'fixtures', 'simple.json')); const transformer = new VDOM(mdast, action.secrets); - transformer.match(({ type }) => type === 'heading', () => '

All Headings are the same to me

'); + transformer.match(({ type }) => type === 'heading', () => '

All Headings are the same to me

'); assertTransformerYieldsDocument( transformer, - '

All Headings are the same to me

', + '

All Headings are the same to me

', ); }); @@ -108,7 +108,7 @@ describe('Test MDAST to VDOM Transformation', () => { assertTransformerYieldsDocument( transformer, `
- +

All Headings are the same to me

`, );