diff --git a/packages/core/src/sanitization/html_sanitizer.ts b/packages/core/src/sanitization/html_sanitizer.ts index eb39f31a13a132..36917d3424acce 100644 --- a/packages/core/src/sanitization/html_sanitizer.ts +++ b/packages/core/src/sanitization/html_sanitizer.ts @@ -83,6 +83,13 @@ const HTML_ATTRS = tagSet( export const VALID_ATTRS = merge(URI_ATTRS, SRCSET_ATTRS, HTML_ATTRS); +// Elements whose content should not be traversed/preserved, if the elements themselves are invalid. +// +// Typically, `Some content` would traverse (and in this case preserve) +// `Some content`, but strip `invalid-element` opening/closing tags. For some elements, though, we +// don't want to preserve the content, if the elements themselves are going to be removed. +const SKIP_TRAVERSING_CONTENT_IF_INVALID_ELEMENTS = tagSet('script,style'); + /** * SanitizingHtmlSerializer serializes a DOM fragment, stripping out any unsafe elements and unsafe * attributes. @@ -98,17 +105,17 @@ class SanitizingHtmlSerializer { // However this code never accesses properties off of `document` before deleting its contents // again, so it shouldn't be vulnerable to DOM clobbering. let current: Node = el.firstChild !; - let elementValid = true; + let traverseContent = true; while (current) { if (current.nodeType === Node.ELEMENT_NODE) { - elementValid = this.startElement(current as Element); + traverseContent = this.startElement(current as Element); } else if (current.nodeType === Node.TEXT_NODE) { this.chars(current.nodeValue !); } else { // Strip non-element, non-text nodes. this.sanitizedSomething = true; } - if (elementValid && current.firstChild) { + if (traverseContent && current.firstChild) { current = current.firstChild !; continue; } @@ -132,18 +139,18 @@ class SanitizingHtmlSerializer { } /** - * Outputs only valid Elements. - * - * Invalid elements are skipped. + * Sanitizes an opening element tag (if valid) and returns whether the element's contents should + * be traversed. Element content must always be traversed (even if the element itself is not + * valid/safe), unless the element is one of `SKIP_TRAVERSING_CONTENT_IF_INVALID_ELEMENTS`. * - * @param element element to sanitize - * Returns true if the element is valid. + * @param element The element to sanitize. + * @return True if the element's contents should be traversed. */ private startElement(element: Element): boolean { const tagName = element.nodeName.toLowerCase(); if (!VALID_ELEMENTS.hasOwnProperty(tagName)) { this.sanitizedSomething = true; - return false; + return !SKIP_TRAVERSING_CONTENT_IF_INVALID_ELEMENTS.hasOwnProperty(tagName); } this.buf.push('<'); this.buf.push(tagName); diff --git a/packages/core/test/sanitization/html_sanitizer_spec.ts b/packages/core/test/sanitization/html_sanitizer_spec.ts index 5f64b0abee92c7..f6f97d34c47541 100644 --- a/packages/core/test/sanitization/html_sanitizer_spec.ts +++ b/packages/core/test/sanitization/html_sanitizer_spec.ts @@ -36,8 +36,8 @@ import {_sanitizeHtml} from '../../src/sanitization/html_sanitizer'; .toEqual('

Hello
World

'); }); - it('supports removal of namespaced elements', - () => { expect(_sanitizeHtml(defaultDoc, 'abc')).toEqual('a'); }); + it('supports namespaced elements', + () => { expect(_sanitizeHtml(defaultDoc, 'abc')).toEqual('abc'); }); it('supports namespaced attributes', () => { expect(_sanitizeHtml(defaultDoc, 't')) @@ -85,31 +85,29 @@ import {_sanitizeHtml} from '../../src/sanitization/html_sanitizer'; .toEqual('

Hello

'); // NB: quote encoded as ASCII ". }); - describe('should strip dangerous elements and its content', () => { + describe('should strip dangerous elements (but traverse its content)', () => { const dangerousTags = [ 'form', + 'frame', 'object', 'textarea', 'button', 'option', 'select', - 'script', - 'style', ]; for (const tag of dangerousTags) { it(`${tag}`, - () => { expect(_sanitizeHtml(defaultDoc, `<${tag}>evil!`)).toEqual(''); }); + () => { expect(_sanitizeHtml(defaultDoc, `<${tag}>evil!`)).toEqual('evil!'); }); } const dangerousSelfClosingTags = [ - 'frameset', - 'embed', - 'input', - 'param', 'base', 'basefont', - 'param', + 'embed', + 'frameset', + 'input', 'link', + 'param', ]; for (const tag of dangerousSelfClosingTags) { @@ -117,10 +115,6 @@ import {_sanitizeHtml} from '../../src/sanitization/html_sanitizer'; expect(_sanitizeHtml(defaultDoc, `before<${tag}>After`)).toEqual('beforeAfter'); }); } - - it(`swallows frame entirely`, () => { - expect(_sanitizeHtml(defaultDoc, `evil!`)).not.toContain(''); - }); }); describe('should strip dangerous attributes', () => { @@ -133,6 +127,13 @@ import {_sanitizeHtml} from '../../src/sanitization/html_sanitizer'; } }); + it('ignores content of script elements', () => { + expect(_sanitizeHtml(defaultDoc, '')).toEqual(''); + expect(_sanitizeHtml(defaultDoc, '
hi
')) + .toEqual('
hi
'); + expect(_sanitizeHtml(defaultDoc, '')).toEqual(''); + }); + it('ignores content of style elements', () => { expect(_sanitizeHtml(defaultDoc, '
hi
')) .toEqual('
hi
');