diff --git a/server/modules/rendering/html-security/renderer.js b/server/modules/rendering/html-security/renderer.js index b57aaf1b8..9a5eca24f 100644 --- a/server/modules/rendering/html-security/renderer.js +++ b/server/modules/rendering/html-security/renderer.js @@ -1,15 +1,35 @@ const { JSDOM } = require('jsdom') const createDOMPurify = require('dompurify') +// Vue directives that are allowed in rendered page content. The page HTML is +// compiled by the Vue template compiler on the client, so any value left on a +// directive would be evaluated as a JavaScript expression. These directives are +// only ever used as valueless flags (e.g. by the tabset renderer), so their +// value is always discarded during sanitization. +const allowedDirectives = ['v-pre', 'v-slot:tabs', 'v-slot:content'] + +// Any attribute the Vue template compiler could pick up as a directive or a +// binding must never carry author-controlled content. +const directiveAttrRegex = /^(v-|:|@|#)/ + module.exports = { async init(input, config) { if (config.safeHTML) { const window = new JSDOM('').window const DOMPurify = createDOMPurify(window) - const allowedAttrs = ['v-pre', 'v-slot:tabs', 'v-slot:content', 'target'] + const allowedAttrs = [...allowedDirectives, 'target'] const allowedTags = ['tabset', 'template'] + DOMPurify.addHook('uponSanitizeAttribute', (elm, data) => { + if (allowedDirectives.includes(data.attrName)) { + // Strip the value so that it can never be compiled as a JS expression + data.attrValue = '' + } else if (directiveAttrRegex.test(data.attrName)) { + data.keepAttr = false + } + }) + if (config.allowDrawIoUnsafe) { allowedTags.push('foreignObject') DOMPurify.addHook('uponSanitizeElement', (elm) => { diff --git a/server/test/modules/rendering/html-security.test.js b/server/test/modules/rendering/html-security.test.js new file mode 100644 index 000000000..4a9da0b7c --- /dev/null +++ b/server/test/modules/rendering/html-security.test.js @@ -0,0 +1,59 @@ +const renderer = require('../../../modules/rendering/html-security/renderer') + +describe('modules/rendering/html-security', () => { + const config = { + safeHTML: true, + allowDrawIoUnsafe: false, + allowIFrames: false + } + + it('keeps the slot directives emitted by the tabset renderer', async () => { + const input = '' + + '' + const result = await renderer.init(input, config) + expect(result).toEqual(input) + }) + + it('strips the value of v-slot:tabs so it cannot be compiled as an expression', async () => { + const input = `` + const result = await renderer.init(input, config) + expect(result).toEqual('') + }) + + it('strips the value of v-slot:content so it cannot be compiled as an expression', async () => { + const input = `` + const result = await renderer.init(input, config) + expect(result).toEqual('') + }) + + it('strips the value of v-pre', async () => { + const result = await renderer.init(`

Text

`, config) + expect(result).toEqual('

Text

') + }) + + it('removes any other directive or binding attribute', async () => { + const inputs = [ + `
Text
`, + `
Text
`, + `
Text
`, + `
Text
`, + `
Text
`, + `
Text
` + ] + for (const input of inputs) { + expect(await renderer.init(input, config)).toEqual('
Text
') + } + }) + + it('leaves regular content attributes untouched', async () => { + const input = 'Link' + const result = await renderer.init(input, config) + expect(result).toEqual(input) + }) + + it('does not sanitize anything when safeHTML is disabled', async () => { + const input = `` + const result = await renderer.init(input, { ...config, safeHTML: false }) + expect(result).toEqual(input) + }) +})