Skip to content

Commit

Permalink
refactor(core): move sanitization into core (angular#22540)
Browse files Browse the repository at this point in the history
This is in preparation of having Ivy have sanitization inline.

PR Close angular#22540
  • Loading branch information
mhevery authored and leo6104 committed Mar 25, 2018
1 parent 1d47661 commit edefe6e
Show file tree
Hide file tree
Showing 17 changed files with 108 additions and 81 deletions.
2 changes: 1 addition & 1 deletion packages/core/src/core.ts
Expand Up @@ -33,7 +33,7 @@ export {EventEmitter} from './event_emitter';
export {ErrorHandler} from './error_handler';
export * from './core_private_export';
export * from './core_render3_private_export';
export {Sanitizer, SecurityContext} from './security';
export {Sanitizer, SecurityContext} from './sanitization/security';
export * from './codegen_private_exports';
export * from './animation/animation_metadata_wrapped';
import {AnimationTriggerMetadata} from './animation/animation_metadata_wrapped';
Expand Down
3 changes: 3 additions & 0 deletions packages/core/src/core_private_export.ts
Expand Up @@ -18,6 +18,9 @@ export {CodegenComponentFactoryResolver as ɵCodegenComponentFactoryResolver} fr
export {ReflectionCapabilities as ɵReflectionCapabilities} from './reflection/reflection_capabilities';
export {GetterFn as ɵGetterFn, MethodFn as ɵMethodFn, SetterFn as ɵSetterFn} from './reflection/types';
export {DirectRenderer as ɵDirectRenderer, RenderDebugInfo as ɵRenderDebugInfo} from './render/api';
export {sanitizeHtml as ɵsanitizeHtml} from './sanitization/html_sanitizer';
export {sanitizeStyle as ɵsanitizeStyle} from './sanitization/style_sanitizer';
export {sanitizeUrl as ɵsanitizeUrl} from './sanitization/url_sanitizer';
export {global as ɵglobal, looseIdentical as ɵlooseIdentical, stringify as ɵstringify} from './util';
export {makeDecorator as ɵmakeDecorator} from './util/decorators';
export {isObservable as ɵisObservable, isPromise as ɵisPromise} from './util/lang';
Expand Down
4 changes: 4 additions & 0 deletions packages/core/src/core_render3_private_export.ts
Expand Up @@ -72,3 +72,7 @@ export {
Pp as ɵPp,
} from './render3/index';
// clang-format on

export {htmlSanitizer as ɵhtmlSanitizer} from './sanitization/html_sanitizer';
export {styleSanitizer as ɵstyleSanitizer} from './sanitization/style_sanitizer';
export {urlSanitizer as ɵurlSanitizer, resourceUrlSanitizer as ɵresourceUrlSanitizer} from './sanitization/url_sanitizer';
Expand Up @@ -8,8 +8,6 @@

import {isDevMode} from '@angular/core';

import {DomAdapter, getDOM} from '../dom/dom_adapter';

import {InertBodyHelper} from './inert_body';
import {sanitizeSrcset, sanitizeUrl} from './url_sanitizer';

Expand Down Expand Up @@ -95,58 +93,61 @@ class SanitizingHtmlSerializer {
// because characters were re-encoded.
public sanitizedSomething = false;
private buf: string[] = [];
private DOM = getDOM();

sanitizeChildren(el: Element): string {
// This cannot use a TreeWalker, as it has to run on Angular's various DOM adapters.
// 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 = this.DOM.firstChild(el) !;
let current: Node = el.firstChild !;
while (current) {
if (this.DOM.isElementNode(current)) {
if (current.nodeType === Node.ELEMENT_NODE) {
this.startElement(current as Element);
} else if (this.DOM.isTextNode(current)) {
this.chars(this.DOM.nodeValue(current) !);
} else if (current.nodeType === Node.TEXT_NODE) {
this.chars(current.nodeValue !);
} else {
// Strip non-element, non-text nodes.
this.sanitizedSomething = true;
}
if (this.DOM.firstChild(current)) {
current = this.DOM.firstChild(current) !;
if (current.firstChild) {
current = current.firstChild !;
continue;
}
while (current) {
// Leaving the element. Walk up and to the right, closing tags as we go.
if (this.DOM.isElementNode(current)) {
if (current.nodeType === Node.ELEMENT_NODE) {
this.endElement(current as Element);
}

let next = this.checkClobberedElement(current, this.DOM.nextSibling(current) !);
let next = this.checkClobberedElement(current, current.nextSibling !);

if (next) {
current = next;
break;
}

current = this.checkClobberedElement(current, this.DOM.parentElement(current) !);
current = this.checkClobberedElement(current, current.parentNode !);
}
}
return this.buf.join('');
}

private startElement(element: Element) {
const tagName = this.DOM.nodeName(element).toLowerCase();
const tagName = element.nodeName.toLowerCase();
if (!VALID_ELEMENTS.hasOwnProperty(tagName)) {
this.sanitizedSomething = true;
return;
}
this.buf.push('<');
this.buf.push(tagName);
this.DOM.attributeMap(element).forEach((value: string, attrName: string) => {
const elAttrs = element.attributes;
for (let i = 0; i < elAttrs.length; i++) {
const elAttr = elAttrs.item(i);
const attrName = elAttr.name;
let value = elAttr.value;
const lower = attrName.toLowerCase();
if (!VALID_ATTRS.hasOwnProperty(lower)) {
this.sanitizedSomething = true;
return;
continue;
}
// TODO(martinprobst): Special case image URIs for data:image/...
if (URI_ATTRS[lower]) value = sanitizeUrl(value);
Expand All @@ -156,12 +157,12 @@ class SanitizingHtmlSerializer {
this.buf.push('="');
this.buf.push(encodeEntities(value));
this.buf.push('"');
});
};
this.buf.push('>');
}

private endElement(current: Element) {
const tagName = this.DOM.nodeName(current).toLowerCase();
const tagName = current.nodeName.toLowerCase();
if (VALID_ELEMENTS.hasOwnProperty(tagName) && !VOID_ELEMENTS.hasOwnProperty(tagName)) {
this.buf.push('</');
this.buf.push(tagName);
Expand All @@ -172,9 +173,9 @@ class SanitizingHtmlSerializer {
private chars(chars: string) { this.buf.push(encodeEntities(chars)); }

checkClobberedElement(node: Node, nextNode: Node): Node {
if (nextNode && this.DOM.contains(node, nextNode)) {
if (nextNode && node.contains(nextNode)) {
throw new Error(
`Failed to sanitize html because the element is clobbered: ${this.DOM.getOuterHTML(node)}`);
`Failed to sanitize html because the element is clobbered: ${(node as Element).outerHTML}`);
}
return nextNode;
}
Expand Down Expand Up @@ -214,10 +215,9 @@ let inertBodyHelper: InertBodyHelper;
* the DOM in a browser environment.
*/
export function sanitizeHtml(defaultDoc: any, unsafeHtmlInput: string): string {
const DOM = getDOM();
let inertBodyElement: HTMLElement|null = null;
try {
inertBodyHelper = inertBodyHelper || new InertBodyHelper(defaultDoc, DOM);
inertBodyHelper = inertBodyHelper || new InertBodyHelper(defaultDoc);
// Make sure unsafeHtml is actually a string (TypeScript types are not enforced at runtime).
let unsafeHtml = unsafeHtmlInput ? String(unsafeHtmlInput) : '';
inertBodyElement = inertBodyHelper.getInertBodyElement(unsafeHtml);
Expand All @@ -234,25 +234,33 @@ export function sanitizeHtml(defaultDoc: any, unsafeHtmlInput: string): string {
mXSSAttempts--;

unsafeHtml = parsedHtml;
parsedHtml = DOM.getInnerHTML(inertBodyElement);
parsedHtml = inertBodyElement !.innerHTML;
inertBodyElement = inertBodyHelper.getInertBodyElement(unsafeHtml);
} while (unsafeHtml !== parsedHtml);

const sanitizer = new SanitizingHtmlSerializer();
const safeHtml =
sanitizer.sanitizeChildren(DOM.getTemplateContent(inertBodyElement) || inertBodyElement);
const safeHtml = sanitizer.sanitizeChildren(
getTemplateContent(inertBodyElement !) as Element || inertBodyElement);
if (isDevMode() && sanitizer.sanitizedSomething) {
DOM.log('WARNING: sanitizing HTML stripped some content (see http://g.co/ng/security#xss).');
console.warn(
'WARNING: sanitizing HTML stripped some content (see http://g.co/ng/security#xss).');
}

return safeHtml;
} finally {
// In case anything goes wrong, clear out inertElement to reset the entire DOM structure.
if (inertBodyElement) {
const parent = DOM.getTemplateContent(inertBodyElement) || inertBodyElement;
for (const child of DOM.childNodesAsList(parent)) {
DOM.removeChild(parent, child);
const parent = getTemplateContent(inertBodyElement) || inertBodyElement;
while (parent.firstChild) {
parent.removeChild(parent.firstChild);
}
}
}
}

function getTemplateContent(el: Node): Node|null {
return 'content' in el && isTemplateElement(el) ? (<any>el).content : null;
}
function isTemplateElement(el: Node): boolean {
return el.nodeType === Node.ELEMENT_NODE && el.nodeName === 'TEMPLATE';
}
Expand Up @@ -6,8 +6,6 @@
* found in the LICENSE file at https://angular.io/license
*/

import {DomAdapter, getDOM} from '../dom/dom_adapter';

/**
* This helper class is used to get hold of an inert tree of DOM elements containing dirty HTML
* that needs sanitizing.
Expand All @@ -18,31 +16,31 @@ import {DomAdapter, getDOM} from '../dom/dom_adapter';
*/
export class InertBodyHelper {
private inertBodyElement: HTMLElement;
private inertDocument: Document;

constructor(private defaultDoc: any, private DOM: DomAdapter) {
const inertDocument = this.DOM.createHtmlDocument();
this.inertBodyElement = inertDocument.body;
constructor(private defaultDoc: Document) {
this.inertDocument = this.defaultDoc.implementation.createHTMLDocument('sanitization-inert');
this.inertBodyElement = this.inertDocument.body;

if (this.inertBodyElement == null) {
// usually there should be only one body element in the document, but IE doesn't have any, so
// we need to create one.
const inertHtml = this.DOM.createElement('html', inertDocument);
this.inertBodyElement = this.DOM.createElement('body', inertDocument);
this.DOM.appendChild(inertHtml, this.inertBodyElement);
this.DOM.appendChild(inertDocument, inertHtml);
const inertHtml = this.inertDocument.createElement('html');
this.inertDocument.appendChild(inertHtml);
this.inertBodyElement = this.inertDocument.createElement('body');
inertHtml.appendChild(this.inertBodyElement);
}

this.DOM.setInnerHTML(
this.inertBodyElement, '<svg><g onload="this.parentNode.remove()"></g></svg>');
this.inertBodyElement.innerHTML = '<svg><g onload="this.parentNode.remove()"></g></svg>';
if (this.inertBodyElement.querySelector && !this.inertBodyElement.querySelector('svg')) {
// We just hit the Safari 10.1 bug - which allows JS to run inside the SVG G element
// so use the XHR strategy.
this.getInertBodyElement = this.getInertBodyElement_XHR;
return;
}

this.DOM.setInnerHTML(
this.inertBodyElement, '<svg><p><style><img src="</style><img src=x onerror=alert(1)//">');
this.inertBodyElement.innerHTML =
'<svg><p><style><img src="</style><img src=x onerror=alert(1)//">';
if (this.inertBodyElement.querySelector && this.inertBodyElement.querySelector('svg img')) {
// We just hit the Firefox bug - which prevents the inner img JS from being sanitized
// so use the DOMParser strategy, if it is available.
Expand Down Expand Up @@ -118,17 +116,17 @@ export class InertBodyHelper {
*/
private getInertBodyElement_InertDocument(html: string) {
// Prefer using <template> element if supported.
const templateEl = this.DOM.createElement('template');
const templateEl = this.inertDocument.createElement('template');
if ('content' in templateEl) {
this.DOM.setInnerHTML(templateEl, html);
templateEl.innerHTML = html;
return templateEl;
}

this.DOM.setInnerHTML(this.inertBodyElement, html);
this.inertBodyElement.innerHTML = html;

// Support: IE 9-11 only
// strip custom-namespaced attributes on IE<=11
if (this.defaultDoc.documentMode) {
if ((this.defaultDoc as any).documentMode) {
this.stripCustomNsAttrs(this.inertBodyElement);
}

Expand All @@ -144,13 +142,19 @@ export class InertBodyHelper {
* strips them all.
*/
private stripCustomNsAttrs(el: Element) {
this.DOM.attributeMap(el).forEach((_, attrName) => {
const elAttrs = el.attributes;
// loop backwards so that we can support removals.
for (let i = elAttrs.length - 1; 0 < i; i--) {
const attrib = elAttrs.item(i);
const attrName = attrib.name;
if (attrName === 'xmlns:ns1' || attrName.indexOf('ns1:') === 0) {
this.DOM.removeAttribute(el, attrName);
el.removeAttribute(attrName);
}
});
for (const n of this.DOM.childNodesAsList(el)) {
if (this.DOM.isElementNode(n)) this.stripCustomNsAttrs(n as Element);
}
let childNode = el.firstChild;
while (childNode) {
if (childNode.nodeType === Node.ELEMENT_NODE) this.stripCustomNsAttrs(childNode as Element);
childNode = childNode.nextSibling;
}
}
}
Expand Down
10 changes: 10 additions & 0 deletions packages/core/src/sanitization/readme.md
@@ -0,0 +1,10 @@
# Sanitization

This folder contains sanitization related code.


## History

It used to be that sanitization related code used to be in `@angular/platform-browser` since it is platform related. While this is true, in practice the compiler schema is permanently tied to the DOM and hence the fact that sanitizer could in theory be replaced is not used in practice.

In order to better support tree shaking we need to be able to refer to the sanitization functions from the Ivy code. For this reason the code has been moved into the `@angular/core`.
File renamed without changes.
Expand Up @@ -8,8 +8,6 @@

import {isDevMode} from '@angular/core';

import {getDOM} from '../dom/dom_adapter';

import {sanitizeUrl} from './url_sanitizer';


Expand Down Expand Up @@ -98,7 +96,7 @@ export function sanitizeStyle(value: string): string {
}

if (isDevMode()) {
getDOM().log(
console.warn(
`WARNING: sanitizing unsafe style value ${value} (see http://g.co/ng/security#xss).`);
}

Expand Down
Expand Up @@ -8,8 +8,6 @@

import {isDevMode} from '@angular/core';

import {getDOM} from '../dom/dom_adapter';


/**
* A pattern that recognizes a commonly useful subset of URLs that are safe.
Expand Down Expand Up @@ -51,7 +49,7 @@ export function sanitizeUrl(url: string): string {
if (url.match(SAFE_URL_PATTERN) || url.match(DATA_URL_PATTERN)) return url;

if (isDevMode()) {
getDOM().log(`WARNING: sanitizing unsafe URL value ${url} (see http://g.co/ng/security#xss)`);
console.warn(`WARNING: sanitizing unsafe URL value ${url} (see http://g.co/ng/security#xss)`);
}

return 'unsafe:' + url;
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/view/element.ts
Expand Up @@ -7,7 +7,7 @@
*/

import {RendererType2} from '../render/api';
import {SecurityContext} from '../security';
import {SecurityContext} from '../sanitization/security';

import {BindingDef, BindingFlags, ElementData, ElementHandleEventFn, NodeDef, NodeFlags, OutputDef, OutputType, QueryValueType, ViewData, ViewDefinitionFactory, asElementData} from './types';
import {NOOP, calcBindingFlags, checkAndUpdateBinding, dispatchEvent, elementEventFullName, getParentRenderElement, resolveDefinition, resolveRendererType2, splitMatchedQueriesDsl, splitNamespace} from './util';
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/view/services.ts
Expand Up @@ -13,7 +13,7 @@ import {ErrorHandler} from '../error_handler';
import {ComponentFactory} from '../linker/component_factory';
import {NgModuleRef} from '../linker/ng_module_factory';
import {Renderer2, RendererFactory2, RendererStyleFlags2, RendererType2} from '../render/api';
import {Sanitizer} from '../security';
import {Sanitizer} from '../sanitization/security';
import {Type} from '../type';

import {isViewDebugError, viewDestroyedError, viewWrappedDebugError} from './errors';
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/view/types.ts
Expand Up @@ -14,7 +14,7 @@ import {QueryList} from '../linker/query_list';
import {TemplateRef} from '../linker/template_ref';
import {ViewContainerRef} from '../linker/view_container_ref';
import {Renderer2, RendererFactory2, RendererType2} from '../render/api';
import {Sanitizer, SecurityContext} from '../security';
import {Sanitizer, SecurityContext} from '../sanitization/security';
import {Type} from '../type';


Expand Down
Expand Up @@ -8,8 +8,7 @@

import {browserDetection} from '@angular/platform-browser/testing/src/browser_util';

import {getDOM} from '../../src/dom/dom_adapter';
import {sanitizeHtml} from '../../src/security/html_sanitizer';
import {sanitizeHtml} from '../../src/sanitization/html_sanitizer';

{
describe('HTML sanitizer', () => {
Expand All @@ -18,13 +17,13 @@ import {sanitizeHtml} from '../../src/security/html_sanitizer';
let logMsgs: string[];

beforeEach(() => {
defaultDoc = getDOM().supportsDOMEvents() ? document : getDOM().createHtmlDocument();
defaultDoc = document;
logMsgs = [];
originalLog = getDOM().log; // Monkey patch DOM.log.
getDOM().log = (msg) => logMsgs.push(msg);
originalLog = console.warn; // Monkey patch DOM.log.
console.warn = (msg: any) => logMsgs.push(msg);
});

afterEach(() => { getDOM().log = originalLog; });
afterEach(() => { console.warn = originalLog; });

it('serializes nested structures', () => {
expect(sanitizeHtml(defaultDoc, '<div alt="x"><p>a</p>b<b>c<a alt="more">d</a></b>e</div>'))
Expand Down

0 comments on commit edefe6e

Please sign in to comment.