From 2e4d17f3a9fe856f28f4c5c31d3f914df77faca5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mi=C5=A1ko=20Hevery?= Date: Wed, 31 Jul 2019 13:15:50 -0700 Subject: [PATCH] perf(core): make sanitization tree-shakable in Ivy mode (#31934) In VE the `Sanitizer` is always available in `BrowserModule` because the VE retrieves it using injection. In Ivy the injection is optional and we have instructions instead of component definition arrays. The implication of this is that in Ivy the instructions can pull in the sanitizer only when they are working with a property which is known to be unsafe. Because the Injection is optional this works even if no Sanitizer is present. So in Ivy we first use the sanitizer which is pulled in by the instruction, unless one is available through the `Injector` then we use that one instead. This PR does few things: 1) It makes `Sanitizer` optional in Ivy. 2) It makes `DomSanitizer` tree shakable. 3) It aligns the semantics of Ivy `Sanitizer` with that of the Ivy sanitization rules. 4) It refactors `DomSanitizer` to use same functions as Ivy sanitization for consistency. PR Close #31934 --- integration/_payload-limits.json | 2 +- packages/core/src/core.ts | 3 +- packages/core/src/core_private_export.ts | 1 + .../core/src/di/injector_compatibility.ts | 4 +- packages/core/src/render3/component.ts | 2 +- packages/core/src/render3/component_ref.ts | 2 +- .../core/src/render3/instructions/shared.ts | 2 +- packages/core/src/render3/interfaces/view.ts | 2 +- .../core/src/render3/styling_next/bindings.ts | 5 +- .../src/render3/styling_next/instructions.ts | 19 ++- packages/core/src/sanitization/bypass.ts | 157 +++++++++++------- .../core/src/sanitization/sanitization.ts | 25 +-- packages/core/src/sanitization/sanitizer.ts | 25 +++ packages/core/src/sanitization/security.ts | 9 - .../core/src/sanitization/style_sanitizer.ts | 3 +- packages/core/src/view/services.ts | 2 +- packages/core/src/view/types.ts | 3 +- .../core/test/acceptance/host_binding_spec.ts | 25 ++- .../acceptance/view_container_ref_spec.ts | 4 +- .../core/test/render3/component_ref_spec.ts | 2 +- .../core/test/render3/instructions_spec.ts | 31 +--- .../core/test/render3/integration_spec.ts | 3 +- packages/core/test/render3/render_util.ts | 2 +- .../test/sanitization/sanatization_spec.ts | 24 +-- .../src/compiler_factory.ts | 6 +- .../src/private_export.ts | 2 +- packages/platform-browser/src/browser.ts | 15 +- .../platform-browser/src/private_export.ts | 2 +- .../src/security/dom_sanitization_service.ts | 83 +++------ .../test/browser/bootstrap_spec.ts | 15 +- tools/public_api_guard/core/core.d.ts | 3 +- 31 files changed, 269 insertions(+), 214 deletions(-) create mode 100644 packages/core/src/sanitization/sanitizer.ts diff --git a/integration/_payload-limits.json b/integration/_payload-limits.json index 3a2aac7f16..a3fa72d17e 100644 --- a/integration/_payload-limits.json +++ b/integration/_payload-limits.json @@ -21,7 +21,7 @@ "master": { "uncompressed": { "runtime": 1440, - "main": 147528, + "main": 136380, "polyfills": 43567 } } diff --git a/packages/core/src/core.ts b/packages/core/src/core.ts index a21cfa7bfc..c87a875d83 100644 --- a/packages/core/src/core.ts +++ b/packages/core/src/core.ts @@ -34,5 +34,6 @@ 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 './sanitization/security'; +export {SecurityContext} from './sanitization/security'; +export {Sanitizer} from './sanitization/sanitizer'; export * from './codegen_private_exports'; diff --git a/packages/core/src/core_private_export.ts b/packages/core/src/core_private_export.ts index fa70f17633..f3978a1cbe 100644 --- a/packages/core/src/core_private_export.ts +++ b/packages/core/src/core_private_export.ts @@ -36,3 +36,4 @@ export {clearOverrides as ɵclearOverrides, initServicesIfNeeded as ɵinitServic export {NOT_FOUND_CHECK_ONLY_ELEMENT_INJECTOR as ɵNOT_FOUND_CHECK_ONLY_ELEMENT_INJECTOR} from './view/provider'; export {getLocalePluralCase as ɵgetLocalePluralCase, findLocaleData as ɵfindLocaleData} from './i18n/locale_data_api'; export {LOCALE_DATA as ɵLOCALE_DATA, LocaleDataIndex as ɵLocaleDataIndex} from './i18n/locale_data'; +export {allowSanitizationBypassAndThrow as ɵallowSanitizationBypassAndThrow, getSanitizationBypassType as ɵgetSanitizationBypassType, BypassType as ɵBypassType, unwrapSafeValue as ɵunwrapSafeValue, SafeHtml as ɵSafeHtml, SafeResourceUrl as ɵSafeResourceUrl, SafeScript as ɵSafeScript, SafeStyle as ɵSafeStyle, SafeUrl as ɵSafeUrl, SafeValue as ɵSafeValue} from './sanitization/bypass'; diff --git a/packages/core/src/di/injector_compatibility.ts b/packages/core/src/di/injector_compatibility.ts index dd78ea322e..096122c0a9 100644 --- a/packages/core/src/di/injector_compatibility.ts +++ b/packages/core/src/di/injector_compatibility.ts @@ -111,7 +111,7 @@ export function ɵɵinject(token: Type| InjectionToken): T; export function ɵɵinject(token: Type| InjectionToken, flags?: InjectFlags): T|null; export function ɵɵinject(token: Type| InjectionToken, flags = InjectFlags.Default): T| null { - return (_injectImplementation || injectInjectorOnly)(token, flags); + return (_injectImplementation || injectInjectorOnly)(resolveForwardRef(token), flags); } /** @@ -201,7 +201,7 @@ export class NullInjector implements Injector { // Intentionally left behind: With dev tools open the debugger will stop here. There is no // reason why correctly written application should cause this exception. // TODO(misko): uncomment the next line once `ngDevMode` works with closure. - // if(ngDevMode) debugger; + // if (ngDevMode) debugger; const error = new Error(`NullInjectorError: No provider for ${stringify(token)}!`); error.name = 'NullInjectorError'; throw error; diff --git a/packages/core/src/render3/component.ts b/packages/core/src/render3/component.ts index 2d4f94ae0f..42525c6597 100644 --- a/packages/core/src/render3/component.ts +++ b/packages/core/src/render3/component.ts @@ -10,7 +10,7 @@ // correctly implementing its interfaces for backwards compatibility. import {Type} from '../core'; import {Injector} from '../di/injector'; -import {Sanitizer} from '../sanitization/security'; +import {Sanitizer} from '../sanitization/sanitizer'; import {assertDataInRange} from '../util/assert'; import {assertComponentType} from './assert'; diff --git a/packages/core/src/render3/component_ref.ts b/packages/core/src/render3/component_ref.ts index 4e64d9a310..469b3b1b85 100644 --- a/packages/core/src/render3/component_ref.ts +++ b/packages/core/src/render3/component_ref.ts @@ -16,7 +16,7 @@ import {ComponentFactoryResolver as viewEngine_ComponentFactoryResolver} from '. import {ElementRef as viewEngine_ElementRef} from '../linker/element_ref'; import {NgModuleRef as viewEngine_NgModuleRef} from '../linker/ng_module_factory'; import {RendererFactory2} from '../render/api'; -import {Sanitizer} from '../sanitization/security'; +import {Sanitizer} from '../sanitization/sanitizer'; import {VERSION} from '../version'; import {NOT_FOUND_CHECK_ONLY_ELEMENT_INJECTOR} from '../view/provider'; diff --git a/packages/core/src/render3/instructions/shared.ts b/packages/core/src/render3/instructions/shared.ts index c532d8a128..9f8fc6215c 100644 --- a/packages/core/src/render3/instructions/shared.ts +++ b/packages/core/src/render3/instructions/shared.ts @@ -9,7 +9,7 @@ import {Injector} from '../../di'; import {ErrorHandler} from '../../error_handler'; import {CUSTOM_ELEMENTS_SCHEMA, NO_ERRORS_SCHEMA, SchemaMetadata} from '../../metadata/schema'; import {validateAgainstEventAttributes, validateAgainstEventProperties} from '../../sanitization/sanitization'; -import {Sanitizer} from '../../sanitization/security'; +import {Sanitizer} from '../../sanitization/sanitizer'; import {assertDataInRange, assertDefined, assertDomNode, assertEqual, assertGreaterThan, assertNotEqual, assertNotSame} from '../../util/assert'; import {createNamedArrayType} from '../../util/named_array_type'; import {normalizeDebugBindingName, normalizeDebugBindingValue} from '../../util/ng_reflect'; diff --git a/packages/core/src/render3/interfaces/view.ts b/packages/core/src/render3/interfaces/view.ts index 10043563c2..b00704aec3 100644 --- a/packages/core/src/render3/interfaces/view.ts +++ b/packages/core/src/render3/interfaces/view.ts @@ -10,7 +10,7 @@ import {InjectionToken} from '../../di/injection_token'; import {Injector} from '../../di/injector'; import {Type} from '../../interface/type'; import {SchemaMetadata} from '../../metadata'; -import {Sanitizer} from '../../sanitization/security'; +import {Sanitizer} from '../../sanitization/sanitizer'; import {LContainer} from './container'; import {ComponentDef, ComponentTemplate, DirectiveDef, DirectiveDefList, HostBindingsFunction, PipeDef, PipeDefList, ViewQueriesFunction} from './definition'; diff --git a/packages/core/src/render3/styling_next/bindings.ts b/packages/core/src/render3/styling_next/bindings.ts index 5f1838dfa3..b92a842f57 100644 --- a/packages/core/src/render3/styling_next/bindings.ts +++ b/packages/core/src/render3/styling_next/bindings.ts @@ -5,6 +5,7 @@ * Use of this source code is governed by an MIT-style license that can be * found in the LICENSE file at https://angular.io/license */ +import {SafeValue} from '../../sanitization/bypass'; import {StyleSanitizeFn, StyleSanitizeMode} from '../../sanitization/style_sanitizer'; import {ProceduralRenderer3, RElement, Renderer3, RendererStyleFlags3, isProceduralRenderer} from '../interfaces/renderer'; @@ -110,7 +111,7 @@ export function updateClassBinding( */ export function updateStyleBinding( context: TStylingContext, data: LStylingData, element: RElement, prop: string | null, - bindingIndex: number, value: String | string | number | null | undefined | StylingMapArray, + bindingIndex: number, value: string | number | SafeValue | null | undefined | StylingMapArray, sanitizer: StyleSanitizeFn | null, deferRegistration: boolean, forceUpdate: boolean): boolean { const isMapBased = !prop; const state = getStylingState(element, stateIsPersisted(context)); @@ -149,7 +150,7 @@ export function updateStyleBinding( function updateBindingData( context: TStylingContext, data: LStylingData, counterIndex: number, prop: string | null, bindingIndex: number, - value: string | String | number | boolean | null | undefined | StylingMapArray, + value: string | SafeValue | number | boolean | null | undefined | StylingMapArray, deferRegistration: boolean, forceUpdate: boolean, sanitizationRequired: boolean): boolean { if (!isContextLocked(context)) { if (deferRegistration) { diff --git a/packages/core/src/render3/styling_next/instructions.ts b/packages/core/src/render3/styling_next/instructions.ts index e8f1398216..955ccd7d56 100644 --- a/packages/core/src/render3/styling_next/instructions.ts +++ b/packages/core/src/render3/styling_next/instructions.ts @@ -5,6 +5,7 @@ * Use of this source code is governed by an MIT-style license that can be * found in the LICENSE file at https://angular.io/license */ +import {SafeValue} from '../../sanitization/bypass'; import {StyleSanitizeFn} from '../../sanitization/style_sanitizer'; import {setInputsForProperty} from '../instructions/shared'; import {AttributeMarker, TAttributes, TNode, TNodeType} from '../interfaces/node'; @@ -96,12 +97,12 @@ export function ɵɵstyleSanitizer(sanitizer: StyleSanitizeFn | null): void { * @codeGenApi */ export function ɵɵstyleProp( - prop: string, value: string | number | String | null, suffix?: string | null): void { + prop: string, value: string | number | SafeValue | null, suffix?: string | null): void { stylePropInternal(getSelectedIndex(), prop, value, suffix); } export function stylePropInternal( - elementIndex: number, prop: string, value: string | number | String | null, + elementIndex: number, prop: string, value: string | number | SafeValue | null, suffix?: string | null | undefined) { const lView = getLView(); @@ -161,8 +162,8 @@ export function ɵɵclassProp(className: string, value: boolean | null): void { */ function _stylingProp( elementIndex: number, bindingIndex: number, prop: string, - value: boolean | number | String | string | null | undefined | NO_CHANGE, isClassBased: boolean, - defer: boolean): boolean { + value: boolean | number | SafeValue | string | null | undefined | NO_CHANGE, + isClassBased: boolean, defer: boolean): boolean { const lView = getLView(); const tNode = getTNode(elementIndex, lView); const native = getNativeByTNode(tNode, lView) as RElement; @@ -176,8 +177,8 @@ function _stylingProp( } else { const sanitizer = getCurrentStyleSanitizer(); updated = updateStyleBinding( - getStylesContext(tNode), lView, native, prop, bindingIndex, value as string | null, - sanitizer, defer, false); + getStylesContext(tNode), lView, native, prop, bindingIndex, + value as string | SafeValue | null, sanitizer, defer, false); } } @@ -508,8 +509,8 @@ function getContext(tNode: TNode, isClassBased: boolean) { } function resolveStylePropValue( - value: string | number | String | null | NO_CHANGE, suffix: string | null | undefined): string| - null|undefined|NO_CHANGE { + value: string | number | SafeValue | null | NO_CHANGE, + suffix: string | null | undefined): string|SafeValue|null|undefined|NO_CHANGE { if (value === NO_CHANGE) return value; let resolvedValue: string|null = null; @@ -519,7 +520,7 @@ function resolveStylePropValue( // sanitization entirely (b/c a new string is created) resolvedValue = renderStringify(value) + suffix; } else { - // sanitization happens by dealing with a String value + // sanitization happens by dealing with a string value // this means that the string value will be passed through // into the style rendering later (which is where the value // will be sanitized before it is applied) diff --git a/packages/core/src/sanitization/bypass.ts b/packages/core/src/sanitization/bypass.ts index 62125fbdb8..fbfc88f44c 100644 --- a/packages/core/src/sanitization/bypass.ts +++ b/packages/core/src/sanitization/bypass.ts @@ -6,61 +6,120 @@ * found in the LICENSE file at https://angular.io/license */ -const BRAND = '__SANITIZER_TRUSTED_BRAND__'; +import {assertEqual} from '../util/assert'; + export const enum BypassType { - Url = 'Url', - Html = 'Html', - ResourceUrl = 'ResourceUrl', + Url = 'URL', + Html = 'HTML', + ResourceUrl = 'ResourceURL', Script = 'Script', Style = 'Style', } /** - * A branded trusted string used with sanitization. + * Marker interface for a value that's safe to use in a particular context. * - * See: {@link TrustedHtmlString}, {@link TrustedResourceUrlString}, {@link TrustedScriptString}, - * {@link TrustedStyleString}, {@link TrustedUrlString} + * @publicApi */ -export interface TrustedString extends String { [BRAND]: BypassType; } +export interface SafeValue {} /** - * A branded trusted string used with sanitization of `html` strings. + * Marker interface for a value that's safe to use as HTML. * - * See: {@link bypassSanitizationTrustHtml} and {@link htmlSanitizer}. + * @publicApi */ -export interface TrustedHtmlString extends TrustedString { [BRAND]: BypassType.Html; } +export interface SafeHtml extends SafeValue {} /** - * A branded trusted string used with sanitization of `style` strings. + * Marker interface for a value that's safe to use as style (CSS). * - * See: {@link bypassSanitizationTrustStyle} and {@link styleSanitizer}. + * @publicApi */ -export interface TrustedStyleString extends TrustedString { [BRAND]: BypassType.Style; } +export interface SafeStyle extends SafeValue {} /** - * A branded trusted string used with sanitization of `url` strings. + * Marker interface for a value that's safe to use as JavaScript. * - * See: {@link bypassSanitizationTrustScript} and {@link scriptSanitizer}. + * @publicApi */ -export interface TrustedScriptString extends TrustedString { [BRAND]: BypassType.Script; } +export interface SafeScript extends SafeValue {} /** - * A branded trusted string used with sanitization of `url` strings. + * Marker interface for a value that's safe to use as a URL linking to a document. * - * See: {@link bypassSanitizationTrustUrl} and {@link urlSanitizer}. + * @publicApi */ -export interface TrustedUrlString extends TrustedString { [BRAND]: BypassType.Url; } +export interface SafeUrl extends SafeValue {} /** - * A branded trusted string used with sanitization of `resourceUrl` strings. + * Marker interface for a value that's safe to use as a URL to load executable code from. * - * See: {@link bypassSanitizationTrustResourceUrl} and {@link resourceUrlSanitizer}. + * @publicApi */ -export interface TrustedResourceUrlString extends TrustedString { [BRAND]: BypassType.ResourceUrl; } +export interface SafeResourceUrl extends SafeValue {} -export function allowSanitizationBypass(value: any, type: BypassType): boolean { - return (value instanceof String && (value as TrustedStyleString)[BRAND] === type); + +abstract class SafeValueImpl implements SafeValue { + constructor(public changingThisBreaksApplicationSecurity: string) { + // empty + } + + abstract getTypeName(): string; + + toString() { + return `SafeValue must use [property]=binding: ${this.changingThisBreaksApplicationSecurity}` + + ` (see http://g.co/ng/security#xss)`; + } +} + +class SafeHtmlImpl extends SafeValueImpl implements SafeHtml { + getTypeName() { return BypassType.Html; } +} +class SafeStyleImpl extends SafeValueImpl implements SafeStyle { + getTypeName() { return BypassType.Style; } +} +class SafeScriptImpl extends SafeValueImpl implements SafeScript { + getTypeName() { return BypassType.Script; } +} +class SafeUrlImpl extends SafeValueImpl implements SafeUrl { + getTypeName() { return BypassType.Url; } +} +class SafeResourceUrlImpl extends SafeValueImpl implements SafeResourceUrl { + getTypeName() { return BypassType.ResourceUrl; } +} + +export function unwrapSafeValue(value: SafeValue): string { + return value instanceof SafeValueImpl ? + (value as SafeValueImpl).changingThisBreaksApplicationSecurity : + ''; +} + + +export function allowSanitizationBypassAndThrow( + value: any, type: BypassType.Html): value is SafeHtml; +export function allowSanitizationBypassAndThrow( + value: any, type: BypassType.ResourceUrl): value is SafeResourceUrl; +export function allowSanitizationBypassAndThrow( + value: any, type: BypassType.Script): value is SafeScript; +export function allowSanitizationBypassAndThrow( + value: any, type: BypassType.Style): value is SafeStyle; +export function allowSanitizationBypassAndThrow(value: any, type: BypassType.Url): value is SafeUrl; +export function allowSanitizationBypassAndThrow(value: any, type: BypassType): boolean; +export function allowSanitizationBypassAndThrow(value: any, type: BypassType): boolean { + const actualType = getSanitizationBypassType(value); + if (actualType != null && actualType !== type) { + // Allow ResourceURLs in URL contexts, they are strictly more trusted. + if (actualType === BypassType.ResourceUrl && type === BypassType.Url) return true; + throw new Error( + `Required a safe ${type}, got a ${actualType} (see http://g.co/ng/security#xss)`); + } + return actualType === type; +} + +export function getSanitizationBypassType(value: any): BypassType|null { + return value instanceof SafeValueImpl && (value as SafeValueImpl).getTypeName() as BypassType || + null; } /** @@ -70,10 +129,10 @@ export function allowSanitizationBypass(value: any, type: BypassType): boolean { * recognizable to {@link htmlSanitizer} to be trusted implicitly. * * @param trustedHtml `html` string which needs to be implicitly trusted. - * @returns a `html` `String` which has been branded to be implicitly trusted. + * @returns a `html` which has been branded to be implicitly trusted. */ -export function bypassSanitizationTrustHtml(trustedHtml: string): TrustedHtmlString { - return bypassSanitizationTrustString(trustedHtml, BypassType.Html); +export function bypassSanitizationTrustHtml(trustedHtml: string): SafeHtml { + return new SafeHtmlImpl(trustedHtml); } /** * Mark `style` string as trusted. @@ -82,10 +141,10 @@ export function bypassSanitizationTrustHtml(trustedHtml: string): TrustedHtmlStr * recognizable to {@link styleSanitizer} to be trusted implicitly. * * @param trustedStyle `style` string which needs to be implicitly trusted. - * @returns a `style` `String` which has been branded to be implicitly trusted. + * @returns a `style` hich has been branded to be implicitly trusted. */ -export function bypassSanitizationTrustStyle(trustedStyle: string): TrustedStyleString { - return bypassSanitizationTrustString(trustedStyle, BypassType.Style); +export function bypassSanitizationTrustStyle(trustedStyle: string): SafeStyle { + return new SafeStyleImpl(trustedStyle); } /** * Mark `script` string as trusted. @@ -94,10 +153,10 @@ export function bypassSanitizationTrustStyle(trustedStyle: string): TrustedStyle * recognizable to {@link scriptSanitizer} to be trusted implicitly. * * @param trustedScript `script` string which needs to be implicitly trusted. - * @returns a `script` `String` which has been branded to be implicitly trusted. + * @returns a `script` which has been branded to be implicitly trusted. */ -export function bypassSanitizationTrustScript(trustedScript: string): TrustedScriptString { - return bypassSanitizationTrustString(trustedScript, BypassType.Script); +export function bypassSanitizationTrustScript(trustedScript: string): SafeScript { + return new SafeScriptImpl(trustedScript); } /** * Mark `url` string as trusted. @@ -106,10 +165,10 @@ export function bypassSanitizationTrustScript(trustedScript: string): TrustedScr * recognizable to {@link urlSanitizer} to be trusted implicitly. * * @param trustedUrl `url` string which needs to be implicitly trusted. - * @returns a `url` `String` which has been branded to be implicitly trusted. + * @returns a `url` which has been branded to be implicitly trusted. */ -export function bypassSanitizationTrustUrl(trustedUrl: string): TrustedUrlString { - return bypassSanitizationTrustString(trustedUrl, BypassType.Url); +export function bypassSanitizationTrustUrl(trustedUrl: string): SafeUrl { + return new SafeUrlImpl(trustedUrl); } /** * Mark `url` string as trusted. @@ -118,26 +177,8 @@ export function bypassSanitizationTrustUrl(trustedUrl: string): TrustedUrlString * recognizable to {@link resourceUrlSanitizer} to be trusted implicitly. * * @param trustedResourceUrl `url` string which needs to be implicitly trusted. - * @returns a `url` `String` which has been branded to be implicitly trusted. + * @returns a `url` which has been branded to be implicitly trusted. */ -export function bypassSanitizationTrustResourceUrl(trustedResourceUrl: string): - TrustedResourceUrlString { - return bypassSanitizationTrustString(trustedResourceUrl, BypassType.ResourceUrl); -} - - -function bypassSanitizationTrustString( - trustedString: string, mode: BypassType.Html): TrustedHtmlString; -function bypassSanitizationTrustString( - trustedString: string, mode: BypassType.Style): TrustedStyleString; -function bypassSanitizationTrustString( - trustedString: string, mode: BypassType.Script): TrustedScriptString; -function bypassSanitizationTrustString( - trustedString: string, mode: BypassType.Url): TrustedUrlString; -function bypassSanitizationTrustString( - trustedString: string, mode: BypassType.ResourceUrl): TrustedResourceUrlString; -function bypassSanitizationTrustString(trustedString: string, mode: BypassType): TrustedString { - const trusted = new String(trustedString) as TrustedString; - trusted[BRAND] = mode; - return trusted; +export function bypassSanitizationTrustResourceUrl(trustedResourceUrl: string): SafeResourceUrl { + return new SafeResourceUrlImpl(trustedResourceUrl); } diff --git a/packages/core/src/sanitization/sanitization.ts b/packages/core/src/sanitization/sanitization.ts index 92953d694a..6f655c1274 100644 --- a/packages/core/src/sanitization/sanitization.ts +++ b/packages/core/src/sanitization/sanitization.ts @@ -10,9 +10,10 @@ import {SANITIZER} from '../render3/interfaces/view'; import {getLView} from '../render3/state'; import {renderStringify} from '../render3/util/misc_utils'; -import {BypassType, allowSanitizationBypass} from './bypass'; +import {BypassType, allowSanitizationBypassAndThrow, unwrapSafeValue} from './bypass'; import {_sanitizeHtml as _sanitizeHtml} from './html_sanitizer'; -import {Sanitizer, SecurityContext} from './security'; +import {Sanitizer} from './sanitizer'; +import {SecurityContext} from './security'; import {StyleSanitizeFn, StyleSanitizeMode, _sanitizeStyle as _sanitizeStyle} from './style_sanitizer'; import {_sanitizeUrl as _sanitizeUrl} from './url_sanitizer'; @@ -38,8 +39,8 @@ export function ɵɵsanitizeHtml(unsafeHtml: any): string { if (sanitizer) { return sanitizer.sanitize(SecurityContext.HTML, unsafeHtml) || ''; } - if (allowSanitizationBypass(unsafeHtml, BypassType.Html)) { - return unsafeHtml.toString(); + if (allowSanitizationBypassAndThrow(unsafeHtml, BypassType.Html)) { + return unwrapSafeValue(unsafeHtml); } return _sanitizeHtml(document, renderStringify(unsafeHtml)); } @@ -64,8 +65,8 @@ export function ɵɵsanitizeStyle(unsafeStyle: any): string { if (sanitizer) { return sanitizer.sanitize(SecurityContext.STYLE, unsafeStyle) || ''; } - if (allowSanitizationBypass(unsafeStyle, BypassType.Style)) { - return unsafeStyle.toString(); + if (allowSanitizationBypassAndThrow(unsafeStyle, BypassType.Style)) { + return unwrapSafeValue(unsafeStyle); } return _sanitizeStyle(renderStringify(unsafeStyle)); } @@ -91,8 +92,8 @@ export function ɵɵsanitizeUrl(unsafeUrl: any): string { if (sanitizer) { return sanitizer.sanitize(SecurityContext.URL, unsafeUrl) || ''; } - if (allowSanitizationBypass(unsafeUrl, BypassType.Url)) { - return unsafeUrl.toString(); + if (allowSanitizationBypassAndThrow(unsafeUrl, BypassType.Url)) { + return unwrapSafeValue(unsafeUrl); } return _sanitizeUrl(renderStringify(unsafeUrl)); } @@ -113,8 +114,8 @@ export function ɵɵsanitizeResourceUrl(unsafeResourceUrl: any): string { if (sanitizer) { return sanitizer.sanitize(SecurityContext.RESOURCE_URL, unsafeResourceUrl) || ''; } - if (allowSanitizationBypass(unsafeResourceUrl, BypassType.ResourceUrl)) { - return unsafeResourceUrl.toString(); + if (allowSanitizationBypassAndThrow(unsafeResourceUrl, BypassType.ResourceUrl)) { + return unwrapSafeValue(unsafeResourceUrl); } throw new Error('unsafe value used in a resource URL context (see http://g.co/ng/security#xss)'); } @@ -136,8 +137,8 @@ export function ɵɵsanitizeScript(unsafeScript: any): string { if (sanitizer) { return sanitizer.sanitize(SecurityContext.SCRIPT, unsafeScript) || ''; } - if (allowSanitizationBypass(unsafeScript, BypassType.Script)) { - return unsafeScript.toString(); + if (allowSanitizationBypassAndThrow(unsafeScript, BypassType.Script)) { + return unwrapSafeValue(unsafeScript); } throw new Error('unsafe value used in a script context'); } diff --git a/packages/core/src/sanitization/sanitizer.ts b/packages/core/src/sanitization/sanitizer.ts new file mode 100644 index 0000000000..b7c21fa4d6 --- /dev/null +++ b/packages/core/src/sanitization/sanitizer.ts @@ -0,0 +1,25 @@ +/** + * @license + * Copyright Google Inc. All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.io/license + */ + +import {ɵɵdefineInjectable} from '../di/interface/defs'; +import {SecurityContext} from './security'; + +/** + * Sanitizer is used by the views to sanitize potentially dangerous values. + * + * @publicApi + */ +export abstract class Sanitizer { + abstract sanitize(context: SecurityContext, value: {}|string|null): string|null; + /** @nocollapse */ + static ngInjectableDef = ɵɵdefineInjectable({ + token: Sanitizer, + providedIn: 'root', + factory: () => null, + }); +} diff --git a/packages/core/src/sanitization/security.ts b/packages/core/src/sanitization/security.ts index 7bfdd9bc7c..b871aec158 100644 --- a/packages/core/src/sanitization/security.ts +++ b/packages/core/src/sanitization/security.ts @@ -23,12 +23,3 @@ export enum SecurityContext { URL = 4, RESOURCE_URL = 5, } - -/** - * Sanitizer is used by the views to sanitize potentially dangerous values. - * - * @publicApi - */ -export abstract class Sanitizer { - abstract sanitize(context: SecurityContext, value: {}|string|null): string|null; -} diff --git a/packages/core/src/sanitization/style_sanitizer.ts b/packages/core/src/sanitization/style_sanitizer.ts index a85746cf0e..835381929b 100644 --- a/packages/core/src/sanitization/style_sanitizer.ts +++ b/packages/core/src/sanitization/style_sanitizer.ts @@ -7,6 +7,7 @@ */ import {isDevMode} from '../util/is_dev_mode'; +import {SafeValue} from './bypass'; import {_sanitizeUrl} from './url_sanitizer'; @@ -135,5 +136,5 @@ export const enum StyleSanitizeMode { * If a value is provided then the sanitized version of that will be returned. */ export interface StyleSanitizeFn { - (prop: string, value: string|null, mode?: StyleSanitizeMode): any; + (prop: string, value: string|SafeValue|null, mode?: StyleSanitizeMode): any; } diff --git a/packages/core/src/view/services.ts b/packages/core/src/view/services.ts index 1ec512853b..15f859f24b 100644 --- a/packages/core/src/view/services.ts +++ b/packages/core/src/view/services.ts @@ -15,7 +15,7 @@ import {Type} from '../interface/type'; import {ComponentFactory} from '../linker/component_factory'; import {NgModuleRef} from '../linker/ng_module_factory'; import {Renderer2, RendererFactory2, RendererStyleFlags2, RendererType2} from '../render/api'; -import {Sanitizer} from '../sanitization/security'; +import {Sanitizer} from '../sanitization/sanitizer'; import {isDevMode} from '../util/is_dev_mode'; import {normalizeDebugBindingName, normalizeDebugBindingValue} from '../util/ng_reflect'; diff --git a/packages/core/src/view/types.ts b/packages/core/src/view/types.ts index 0f43c13274..004636d66a 100644 --- a/packages/core/src/view/types.ts +++ b/packages/core/src/view/types.ts @@ -15,7 +15,8 @@ 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 '../sanitization/security'; +import {Sanitizer} from '../sanitization/sanitizer'; +import {SecurityContext} from '../sanitization/security'; diff --git a/packages/core/test/acceptance/host_binding_spec.ts b/packages/core/test/acceptance/host_binding_spec.ts index 4e0d3e17b0..5ac5d32c0a 100644 --- a/packages/core/test/acceptance/host_binding_spec.ts +++ b/packages/core/test/acceptance/host_binding_spec.ts @@ -8,7 +8,7 @@ import {CommonModule} from '@angular/common'; import {AfterContentInit, Component, ComponentFactoryResolver, ComponentRef, ContentChildren, Directive, DoCheck, HostBinding, HostListener, Injectable, Input, NgModule, OnChanges, OnInit, QueryList, ViewChild, ViewChildren, ViewContainerRef} from '@angular/core'; -import {bypassSanitizationTrustHtml, bypassSanitizationTrustUrl} from '@angular/core/src/sanitization/bypass'; +import {bypassSanitizationTrustHtml, bypassSanitizationTrustStyle, bypassSanitizationTrustUrl} from '@angular/core/src/sanitization/bypass'; import {TestBed} from '@angular/core/testing'; import {ivyEnabled, onlyInIvy} from '@angular/private/testing'; @@ -1021,10 +1021,11 @@ describe('host bindings', () => { }); describe('sanitization', () => { + function identity(value: any) { return value; } function verify( - tag: string, prop: string, value: any, expectedSanitizedValue: any, bypassFn: any, - isAttribute: boolean = true) { - it('should sanitize potentially unsafe properties and attributes', () => { + tag: string, prop: string, value: any, expectedSanitizedValue: any, bypassFn: Function, + isAttribute: boolean = true, throws: boolean = false) { + it(`should sanitize <${tag} ${prop}> ${isAttribute ? 'properties' : 'attributes'}`, () => { @Directive({ selector: '[unsafeUrlHostBindingDir]', host: { @@ -1051,17 +1052,29 @@ describe('host bindings', () => { expect(current()).toEqual(expectedSanitizedValue); fixture.componentInstance.unsafeDir.value = bypassFn(value); - fixture.detectChanges(); - expect(current()).toEqual(expectedSanitizedValue); + if (throws) { + expect(() => fixture.detectChanges()).toThrowError(/Required a safe URL, got a \w+/); + } else { + fixture.detectChanges(); + expect(current()).toEqual(bypassFn == identity ? expectedSanitizedValue : value); + } }); } verify( 'a', 'href', 'javascript:alert(1)', 'unsafe:javascript:alert(1)', bypassSanitizationTrustUrl); + verify('a', 'href', 'javascript:alert(1.1)', 'unsafe:javascript:alert(1.1)', identity); + verify( + 'a', 'href', 'javascript:alert(1.2)', 'unsafe:javascript:alert(1.2)', + bypassSanitizationTrustStyle, true, true); verify( 'blockquote', 'cite', 'javascript:alert(2)', 'unsafe:javascript:alert(2)', bypassSanitizationTrustUrl); + verify('blockquote', 'cite', 'javascript:alert(2.1)', 'unsafe:javascript:alert(2.1)', identity); + verify( + 'blockquote', 'cite', 'javascript:alert(2.2)', 'unsafe:javascript:alert(2.2)', + bypassSanitizationTrustHtml, true, true); verify( 'b', 'innerHTML', '', '', bypassSanitizationTrustHtml, diff --git a/packages/core/test/acceptance/view_container_ref_spec.ts b/packages/core/test/acceptance/view_container_ref_spec.ts index 0d19108150..b73e2c26f5 100644 --- a/packages/core/test/acceptance/view_container_ref_spec.ts +++ b/packages/core/test/acceptance/view_container_ref_spec.ts @@ -11,7 +11,7 @@ import {Compiler, Component, ComponentFactoryResolver, Directive, DoCheck, Eleme import {Input} from '@angular/core/src/metadata'; import {ngDevModeResetPerfCounters} from '@angular/core/src/util/ng_dev_mode'; import {TestBed, TestComponentRenderer} from '@angular/core/testing'; -import {By} from '@angular/platform-browser'; +import {By, DomSanitizer} from '@angular/platform-browser'; import {expect} from '@angular/platform-browser/testing/src/matchers'; import {ivyEnabled, onlyInIvy} from '@angular/private/testing'; @@ -943,7 +943,7 @@ describe('ViewContainerRef', () => { {provide: String, useValue: 'root_module'}, // We need to provide the following tokens because otherwise view engine // will throw when creating a component factory in debug mode. - {provide: Sanitizer, useValue: TestBed.get(Sanitizer)}, + {provide: Sanitizer, useValue: TestBed.get(DomSanitizer)}, {provide: ErrorHandler, useValue: TestBed.get(ErrorHandler)}, {provide: RendererFactory2, useValue: TestBed.get(RendererFactory2)}, ] diff --git a/packages/core/test/render3/component_ref_spec.ts b/packages/core/test/render3/component_ref_spec.ts index 71c61c6d4f..cf85556cf2 100644 --- a/packages/core/test/render3/component_ref_spec.ts +++ b/packages/core/test/render3/component_ref_spec.ts @@ -12,7 +12,7 @@ import {RendererFactory2} from '../../src/render/api'; import {injectComponentFactoryResolver} from '../../src/render3/component_ref'; import {ɵɵdefineComponent} from '../../src/render3/index'; import {domRendererFactory3} from '../../src/render3/interfaces/renderer'; -import {Sanitizer} from '../../src/sanitization/security'; +import {Sanitizer} from '../../src/sanitization/sanitizer'; describe('ComponentFactory', () => { const cfr = injectComponentFactoryResolver(); diff --git a/packages/core/test/render3/instructions_spec.ts b/packages/core/test/render3/instructions_spec.ts index aed1eaf4eb..7f7da03ece 100644 --- a/packages/core/test/render3/instructions_spec.ts +++ b/packages/core/test/render3/instructions_spec.ts @@ -11,9 +11,10 @@ import {NgForOfContext} from '@angular/common'; import {ɵɵdefineComponent} from '../../src/render3/definition'; import {RenderFlags, ɵɵattribute, ɵɵclassMap, ɵɵelement, ɵɵelementEnd, ɵɵelementStart, ɵɵproperty, ɵɵselect, ɵɵstyleMap, ɵɵstyleProp, ɵɵstyleSanitizer, ɵɵstyling, ɵɵstylingApply, ɵɵtemplate, ɵɵtext, ɵɵtextInterpolate1} from '../../src/render3/index'; import {AttributeMarker} from '../../src/render3/interfaces/node'; -import {bypassSanitizationTrustHtml, bypassSanitizationTrustResourceUrl, bypassSanitizationTrustScript, bypassSanitizationTrustStyle, bypassSanitizationTrustUrl} from '../../src/sanitization/bypass'; +import {bypassSanitizationTrustHtml, bypassSanitizationTrustResourceUrl, bypassSanitizationTrustScript, bypassSanitizationTrustStyle, bypassSanitizationTrustUrl, getSanitizationBypassType, unwrapSafeValue} from '../../src/sanitization/bypass'; import {ɵɵdefaultStyleSanitizer, ɵɵsanitizeHtml, ɵɵsanitizeResourceUrl, ɵɵsanitizeScript, ɵɵsanitizeStyle, ɵɵsanitizeUrl} from '../../src/sanitization/sanitization'; -import {Sanitizer, SecurityContext} from '../../src/sanitization/security'; +import {Sanitizer} from '../../src/sanitization/sanitizer'; +import {SecurityContext} from '../../src/sanitization/security'; import {NgForOf} from './common_with_def'; import {ComponentFixture, TemplateFixture} from './render_util'; @@ -161,33 +162,13 @@ describe('instructions', () => { expect(t.html).toEqual('
'); t.update(() => { + ɵɵstyleSanitizer(ɵɵdefaultStyleSanitizer); ɵɵstyleProp('background-image', bypassSanitizationTrustStyle('url("http://server2")')); ɵɵstylingApply(); }); expect((t.hostElement.firstChild as HTMLElement).style.getPropertyValue('background-image')) .toEqual('url("http://server2")'); }); - - it('should not re-apply the style value even if it is a newly bypassed again', () => { - const sanitizerInterceptor = new MockSanitizerInterceptor(); - const t = createTemplateFixtureWithSanitizer(() => createDiv(), 1, sanitizerInterceptor); - - t.update(() => { - ɵɵstyleSanitizer(sanitizerInterceptor.getStyleSanitizer()); - ɵɵstyleProp('background-image', bypassSanitizationTrustStyle('apple')); - ɵɵstylingApply(); - }); - - expect(sanitizerInterceptor.lastValue !).toEqual('apple'); - sanitizerInterceptor.lastValue = null; - - t.update(() => { - ɵɵstyleSanitizer(sanitizerInterceptor.getStyleSanitizer()); - ɵɵstyleProp('background-image', bypassSanitizationTrustStyle('apple')); - ɵɵstylingApply(); - }); - expect(sanitizerInterceptor.lastValue).toEqual(null); - }); }); describe('styleMap', () => { @@ -498,8 +479,8 @@ class LocalMockSanitizer implements Sanitizer { constructor(private _interceptor: (value: string|null|any) => string) {} sanitize(context: SecurityContext, value: LocalSanitizedValue|string|null|any): string|null { - if (value instanceof String) { - return value.toString() + '-ivy'; + if (getSanitizationBypassType(value) != null) { + return unwrapSafeValue(value) + '-ivy'; } if (value instanceof LocalSanitizedValue) { diff --git a/packages/core/test/render3/integration_spec.ts b/packages/core/test/render3/integration_spec.ts index d071c9abb2..a4bf661d4c 100644 --- a/packages/core/test/render3/integration_spec.ts +++ b/packages/core/test/render3/integration_spec.ts @@ -15,7 +15,8 @@ import {RenderFlags} from '../../src/render3/interfaces/definition'; import {RElement, Renderer3, RendererFactory3, domRendererFactory3} from '../../src/render3/interfaces/renderer'; import {CONTEXT, HEADER_OFFSET} from '../../src/render3/interfaces/view'; import {ɵɵsanitizeUrl} from '../../src/sanitization/sanitization'; -import {Sanitizer, SecurityContext} from '../../src/sanitization/security'; +import {Sanitizer} from '../../src/sanitization/sanitizer'; +import {SecurityContext} from '../../src/sanitization/security'; import {NgIf} from './common_with_def'; import {ComponentFixture, MockRendererFactory, renderToHtml} from './render_util'; diff --git a/packages/core/test/render3/render_util.ts b/packages/core/test/render3/render_util.ts index 9e0a492bf4..46e8dea6fa 100644 --- a/packages/core/test/render3/render_util.ts +++ b/packages/core/test/render3/render_util.ts @@ -35,7 +35,7 @@ import {ProceduralRenderer3, RComment, RElement, RNode, RText, Renderer3, Render import {HEADER_OFFSET, LView, LViewFlags, TVIEW, T_HOST} from '../../src/render3/interfaces/view'; import {destroyLView} from '../../src/render3/node_manipulation'; import {getRootView} from '../../src/render3/util/view_traversal_utils'; -import {Sanitizer} from '../../src/sanitization/security'; +import {Sanitizer} from '../../src/sanitization/sanitizer'; import {getRendererFactory2} from './imported_renderer2'; diff --git a/packages/core/test/sanitization/sanatization_spec.ts b/packages/core/test/sanitization/sanatization_spec.ts index 6c42ec2120..5d09d21194 100644 --- a/packages/core/test/sanitization/sanatization_spec.ts +++ b/packages/core/test/sanitization/sanatization_spec.ts @@ -33,8 +33,8 @@ describe('sanitization', () => { .toEqual(''); expect(ɵɵsanitizeHtml(new Wrap(''))) .toEqual(''); - expect(ɵɵsanitizeHtml(bypassSanitizationTrustUrl(''))) - .toEqual(''); + expect(() => ɵɵsanitizeHtml(bypassSanitizationTrustUrl(''))) + .toThrowError(/Required a safe HTML, got a URL/); expect(ɵɵsanitizeHtml(bypassSanitizationTrustHtml(''))) .toEqual(''); }); @@ -44,8 +44,8 @@ describe('sanitization', () => { expect(ɵɵsanitizeUrl(new Wrap('http://server'))).toEqual('http://server'); expect(ɵɵsanitizeUrl('javascript:true')).toEqual('unsafe:javascript:true'); expect(ɵɵsanitizeUrl(new Wrap('javascript:true'))).toEqual('unsafe:javascript:true'); - expect(ɵɵsanitizeUrl(bypassSanitizationTrustHtml('javascript:true'))) - .toEqual('unsafe:javascript:true'); + expect(() => ɵɵsanitizeUrl(bypassSanitizationTrustHtml('javascript:true'))) + .toThrowError(/Required a safe URL, got a HTML/); expect(ɵɵsanitizeUrl(bypassSanitizationTrustUrl('javascript:true'))).toEqual('javascript:true'); }); @@ -54,7 +54,7 @@ describe('sanitization', () => { expect(() => ɵɵsanitizeResourceUrl('http://server')).toThrowError(ERROR); expect(() => ɵɵsanitizeResourceUrl('javascript:true')).toThrowError(ERROR); expect(() => ɵɵsanitizeResourceUrl(bypassSanitizationTrustHtml('javascript:true'))) - .toThrowError(ERROR); + .toThrowError(/Required a safe ResourceURL, got a HTML/); expect(ɵɵsanitizeResourceUrl(bypassSanitizationTrustResourceUrl('javascript:true'))) .toEqual('javascript:true'); }); @@ -64,7 +64,8 @@ describe('sanitization', () => { expect(ɵɵsanitizeStyle(new Wrap('red'))).toEqual('red'); expect(ɵɵsanitizeStyle('url("http://server")')).toEqual('unsafe'); expect(ɵɵsanitizeStyle(new Wrap('url("http://server")'))).toEqual('unsafe'); - expect(ɵɵsanitizeStyle(bypassSanitizationTrustHtml('url("http://server")'))).toEqual('unsafe'); + expect(() => ɵɵsanitizeStyle(bypassSanitizationTrustHtml('url("http://server")'))) + .toThrowError(/Required a safe Style, got a HTML/); expect(ɵɵsanitizeStyle(bypassSanitizationTrustStyle('url("http://server")'))) .toEqual('url("http://server")'); }); @@ -73,7 +74,8 @@ describe('sanitization', () => { const ERROR = 'unsafe value used in a script context'; expect(() => ɵɵsanitizeScript('true')).toThrowError(ERROR); expect(() => ɵɵsanitizeScript('true')).toThrowError(ERROR); - expect(() => ɵɵsanitizeScript(bypassSanitizationTrustHtml('true'))).toThrowError(ERROR); + expect(() => ɵɵsanitizeScript(bypassSanitizationTrustHtml('true'))) + .toThrowError(/Required a safe Script, got a HTML/); expect(ɵɵsanitizeScript(bypassSanitizationTrustScript('true'))).toEqual('true'); }); @@ -108,7 +110,7 @@ describe('sanitization', () => { expect( () => ɵɵsanitizeUrlOrResourceUrl( bypassSanitizationTrustHtml('javascript:true'), 'iframe', 'src')) - .toThrowError(ERROR); + .toThrowError(/Required a safe ResourceURL, got a HTML/); expect(ɵɵsanitizeUrlOrResourceUrl( bypassSanitizationTrustResourceUrl('javascript:true'), 'iframe', 'src')) .toEqual('javascript:true'); @@ -122,8 +124,10 @@ describe('sanitization', () => { .toEqual('unsafe:javascript:true'); expect(ɵɵsanitizeUrlOrResourceUrl(new Wrap('javascript:true'), 'a', 'href')) .toEqual('unsafe:javascript:true'); - expect(ɵɵsanitizeUrlOrResourceUrl(bypassSanitizationTrustHtml('javascript:true'), 'a', 'href')) - .toEqual('unsafe:javascript:true'); + expect( + () => + ɵɵsanitizeUrlOrResourceUrl(bypassSanitizationTrustHtml('javascript:true'), 'a', 'href')) + .toThrowError(/Required a safe URL, got a HTML/); expect(ɵɵsanitizeUrlOrResourceUrl(bypassSanitizationTrustUrl('javascript:true'), 'a', 'href')) .toEqual('javascript:true'); }); diff --git a/packages/platform-browser-dynamic/src/compiler_factory.ts b/packages/platform-browser-dynamic/src/compiler_factory.ts index 728117a857..bac7cfdfe4 100644 --- a/packages/platform-browser-dynamic/src/compiler_factory.ts +++ b/packages/platform-browser-dynamic/src/compiler_factory.ts @@ -84,12 +84,11 @@ export class CompilerImpl implements Compiler { return meta && meta.id || undefined; } } - /** * A set of providers that provide `JitCompiler` and its dependencies to use for * template compilation. */ -export const COMPILER_PROVIDERS = [ +const COMPILER_PROVIDERS__PRE_R3__ = [ {provide: CompileReflector, useValue: new JitReflector()}, {provide: ResourceLoader, useValue: _NO_RESOURCE_LOADER}, {provide: JitSummaryResolver, deps: []}, @@ -156,6 +155,9 @@ export const COMPILER_PROVIDERS = [ { provide: NgModuleResolver, deps: [CompileReflector]}, ]; +export const COMPILER_PROVIDERS__POST_R3__ = + [{provide: Compiler, useFactory: () => new Compiler()}]; +export const COMPILER_PROVIDERS = COMPILER_PROVIDERS__PRE_R3__; /** * @publicApi */ diff --git a/packages/platform-browser-dynamic/src/private_export.ts b/packages/platform-browser-dynamic/src/private_export.ts index 42da202adc..63b28f25d5 100644 --- a/packages/platform-browser-dynamic/src/private_export.ts +++ b/packages/platform-browser-dynamic/src/private_export.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ -export {CompilerImpl as ɵCompilerImpl} from './compiler_factory'; +export {COMPILER_PROVIDERS__POST_R3__ as ɵCOMPILER_PROVIDERS__POST_R3__, CompilerImpl as ɵCompilerImpl} from './compiler_factory'; export {platformCoreDynamic as ɵplatformCoreDynamic} from './platform_core_dynamic'; export {INTERNAL_BROWSER_DYNAMIC_PLATFORM_PROVIDERS as ɵINTERNAL_BROWSER_DYNAMIC_PLATFORM_PROVIDERS} from './platform_providers'; export {ResourceLoaderImpl as ɵResourceLoaderImpl} from './resource_loader/resource_loader_impl'; diff --git a/packages/platform-browser/src/browser.ts b/packages/platform-browser/src/browser.ts index 22a246afed..bf8774e5bf 100644 --- a/packages/platform-browser/src/browser.ts +++ b/packages/platform-browser/src/browser.ts @@ -29,16 +29,23 @@ export const INTERNAL_BROWSER_PLATFORM_PROVIDERS: StaticProvider[] = [ {provide: DOCUMENT, useFactory: _document, deps: []}, ]; +const BROWSER_SANITIZATION_PROVIDERS__PRE_R3__: StaticProvider[] = [ + {provide: Sanitizer, useExisting: DomSanitizer}, + {provide: DomSanitizer, useClass: DomSanitizerImpl, deps: [DOCUMENT]}, +]; + +/** + * @codeGenApi + */ +export const BROWSER_SANITIZATION_PROVIDERS__POST_R3__ = []; + /** * @security Replacing built-in sanitization providers exposes the application to XSS risks. * Attacker-controlled data introduced by an unsanitized provider could expose your * application to XSS risks. For more detail, see the [Security Guide](http://g.co/ng/security). * @publicApi */ -export const BROWSER_SANITIZATION_PROVIDERS: StaticProvider[] = [ - {provide: Sanitizer, useExisting: DomSanitizer}, - {provide: DomSanitizer, useClass: DomSanitizerImpl, deps: [DOCUMENT]}, -]; +export const BROWSER_SANITIZATION_PROVIDERS = BROWSER_SANITIZATION_PROVIDERS__PRE_R3__; /** * @publicApi diff --git a/packages/platform-browser/src/private_export.ts b/packages/platform-browser/src/private_export.ts index 0deb396d77..4957e315bc 100644 --- a/packages/platform-browser/src/private_export.ts +++ b/packages/platform-browser/src/private_export.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ -export {BROWSER_SANITIZATION_PROVIDERS as ɵBROWSER_SANITIZATION_PROVIDERS, INTERNAL_BROWSER_PLATFORM_PROVIDERS as ɵINTERNAL_BROWSER_PLATFORM_PROVIDERS, initDomAdapter as ɵinitDomAdapter} from './browser'; +export {BROWSER_SANITIZATION_PROVIDERS as ɵBROWSER_SANITIZATION_PROVIDERS, BROWSER_SANITIZATION_PROVIDERS__POST_R3__ as ɵBROWSER_SANITIZATION_PROVIDERS__POST_R3__, INTERNAL_BROWSER_PLATFORM_PROVIDERS as ɵINTERNAL_BROWSER_PLATFORM_PROVIDERS, initDomAdapter as ɵinitDomAdapter} from './browser'; export {BrowserDomAdapter as ɵBrowserDomAdapter} from './browser/browser_adapter'; export {BrowserPlatformLocation as ɵBrowserPlatformLocation} from './browser/location/browser_platform_location'; export {TRANSITION_ID as ɵTRANSITION_ID} from './browser/server-transition'; diff --git a/packages/platform-browser/src/security/dom_sanitization_service.ts b/packages/platform-browser/src/security/dom_sanitization_service.ts index 12a86bd118..c448588b34 100644 --- a/packages/platform-browser/src/security/dom_sanitization_service.ts +++ b/packages/platform-browser/src/security/dom_sanitization_service.ts @@ -7,8 +7,7 @@ */ import {DOCUMENT} from '@angular/common'; -import {Inject, Injectable, Sanitizer, SecurityContext, ɵ_sanitizeHtml as _sanitizeHtml, ɵ_sanitizeStyle as _sanitizeStyle, ɵ_sanitizeUrl as _sanitizeUrl} from '@angular/core'; - +import {Inject, Injectable, Injector, Sanitizer, SecurityContext, forwardRef, ɵBypassType as BypassType, ɵ_sanitizeHtml as _sanitizeHtml, ɵ_sanitizeStyle as _sanitizeStyle, ɵ_sanitizeUrl as _sanitizeUrl, ɵallowSanitizationBypassAndThrow as allowSanitizationBypassOrThrow, ɵbypassSanitizationTrustHtml as bypassSanitizationTrustHtml, ɵbypassSanitizationTrustResourceUrl as bypassSanitizationTrustResourceUrl, ɵbypassSanitizationTrustScript as bypassSanitizationTrustScript, ɵbypassSanitizationTrustStyle as bypassSanitizationTrustStyle, ɵbypassSanitizationTrustUrl as bypassSanitizationTrustUrl, ɵgetSanitizationBypassType as getSanitizationBypassType, ɵunwrapSafeValue as unwrapSafeValue} from '@angular/core'; export {SecurityContext}; @@ -87,6 +86,7 @@ export interface SafeResourceUrl extends SafeValue {} * * @publicApi */ +@Injectable({providedIn: 'root', useExisting: forwardRef(() => DomSanitizerImpl)}) export abstract class DomSanitizer implements Sanitizer { /** * Sanitizes a value for use in the given SecurityContext. @@ -143,8 +143,11 @@ export abstract class DomSanitizer implements Sanitizer { abstract bypassSecurityTrustResourceUrl(value: string): SafeResourceUrl; } +export function domSanitizerImplFactory(injector: Injector) { + return new DomSanitizerImpl(injector.get(DOCUMENT)); +} -@Injectable() +@Injectable({providedIn: 'root', useFactory: domSanitizerImplFactory, deps: [Injector]}) export class DomSanitizerImpl extends DomSanitizer { constructor(@Inject(DOCUMENT) private _doc: any) { super(); } @@ -154,29 +157,30 @@ export class DomSanitizerImpl extends DomSanitizer { case SecurityContext.NONE: return value as string; case SecurityContext.HTML: - if (value instanceof SafeHtmlImpl) return value.changingThisBreaksApplicationSecurity; - this.checkNotSafeValue(value, 'HTML'); + if (allowSanitizationBypassOrThrow(value, BypassType.Html)) { + return unwrapSafeValue(value); + } return _sanitizeHtml(this._doc, String(value)); case SecurityContext.STYLE: - if (value instanceof SafeStyleImpl) return value.changingThisBreaksApplicationSecurity; - this.checkNotSafeValue(value, 'Style'); + if (allowSanitizationBypassOrThrow(value, BypassType.Style)) { + return unwrapSafeValue(value); + } return _sanitizeStyle(value as string); case SecurityContext.SCRIPT: - if (value instanceof SafeScriptImpl) return value.changingThisBreaksApplicationSecurity; - this.checkNotSafeValue(value, 'Script'); + if (allowSanitizationBypassOrThrow(value, BypassType.Script)) { + return unwrapSafeValue(value); + } throw new Error('unsafe value used in a script context'); case SecurityContext.URL: - if (value instanceof SafeResourceUrlImpl || value instanceof SafeUrlImpl) { - // Allow resource URLs in URL contexts, they are strictly more trusted. - return value.changingThisBreaksApplicationSecurity; + const type = getSanitizationBypassType(value); + if (allowSanitizationBypassOrThrow(value, BypassType.Url)) { + return unwrapSafeValue(value); } - this.checkNotSafeValue(value, 'URL'); return _sanitizeUrl(String(value)); case SecurityContext.RESOURCE_URL: - if (value instanceof SafeResourceUrlImpl) { - return value.changingThisBreaksApplicationSecurity; + if (allowSanitizationBypassOrThrow(value, BypassType.ResourceUrl)) { + return unwrapSafeValue(value); } - this.checkNotSafeValue(value, 'ResourceURL'); throw new Error( 'unsafe value used in a resource URL context (see http://g.co/ng/security#xss)'); default: @@ -184,48 +188,13 @@ export class DomSanitizerImpl extends DomSanitizer { } } - private checkNotSafeValue(value: any, expectedType: string) { - if (value instanceof SafeValueImpl) { - throw new Error( - `Required a safe ${expectedType}, got a ${value.getTypeName()} ` + - `(see http://g.co/ng/security#xss)`); - } + bypassSecurityTrustHtml(value: string): SafeHtml { return bypassSanitizationTrustHtml(value); } + bypassSecurityTrustStyle(value: string): SafeStyle { return bypassSanitizationTrustStyle(value); } + bypassSecurityTrustScript(value: string): SafeScript { + return bypassSanitizationTrustScript(value); } - - bypassSecurityTrustHtml(value: string): SafeHtml { return new SafeHtmlImpl(value); } - bypassSecurityTrustStyle(value: string): SafeStyle { return new SafeStyleImpl(value); } - bypassSecurityTrustScript(value: string): SafeScript { return new SafeScriptImpl(value); } - bypassSecurityTrustUrl(value: string): SafeUrl { return new SafeUrlImpl(value); } + bypassSecurityTrustUrl(value: string): SafeUrl { return bypassSanitizationTrustUrl(value); } bypassSecurityTrustResourceUrl(value: string): SafeResourceUrl { - return new SafeResourceUrlImpl(value); + return bypassSanitizationTrustResourceUrl(value); } } - -abstract class SafeValueImpl implements SafeValue { - constructor(public changingThisBreaksApplicationSecurity: string) { - // empty - } - - abstract getTypeName(): string; - - toString() { - return `SafeValue must use [property]=binding: ${this.changingThisBreaksApplicationSecurity}` + - ` (see http://g.co/ng/security#xss)`; - } -} - -class SafeHtmlImpl extends SafeValueImpl implements SafeHtml { - getTypeName() { return 'HTML'; } -} -class SafeStyleImpl extends SafeValueImpl implements SafeStyle { - getTypeName() { return 'Style'; } -} -class SafeScriptImpl extends SafeValueImpl implements SafeScript { - getTypeName() { return 'Script'; } -} -class SafeUrlImpl extends SafeValueImpl implements SafeUrl { - getTypeName() { return 'URL'; } -} -class SafeResourceUrlImpl extends SafeValueImpl implements SafeResourceUrl { - getTypeName() { return 'ResourceURL'; } -} diff --git a/packages/platform-browser/test/browser/bootstrap_spec.ts b/packages/platform-browser/test/browser/bootstrap_spec.ts index 3a057c3a74..d89d377aea 100644 --- a/packages/platform-browser/test/browser/bootstrap_spec.ts +++ b/packages/platform-browser/test/browser/bootstrap_spec.ts @@ -7,7 +7,7 @@ */ import {DOCUMENT, isPlatformBrowser} from '@angular/common'; -import {APP_INITIALIZER, CUSTOM_ELEMENTS_SCHEMA, Compiler, Component, Directive, ErrorHandler, Inject, Input, LOCALE_ID, NgModule, OnDestroy, PLATFORM_ID, PLATFORM_INITIALIZER, Pipe, Provider, StaticProvider, Type, VERSION, createPlatformFactory} from '@angular/core'; +import {APP_INITIALIZER, CUSTOM_ELEMENTS_SCHEMA, Compiler, Component, Directive, ErrorHandler, Inject, Injector, Input, LOCALE_ID, NgModule, OnDestroy, PLATFORM_ID, PLATFORM_INITIALIZER, Pipe, Provider, Sanitizer, StaticProvider, Type, VERSION, createPlatformFactory} from '@angular/core'; import {ApplicationRef, destroyPlatform} from '@angular/core/src/application_ref'; import {Console} from '@angular/core/src/console'; import {ComponentRef} from '@angular/core/src/linker/component_factory'; @@ -189,6 +189,19 @@ function bootstrap( }); })); + it('should retrieve sanitizer', inject([Injector], (injector: Injector) => { + const sanitizer: Sanitizer|null = injector.get(Sanitizer, null); + if (ivyEnabled) { + // In Ivy we don't want to have sanitizer in DI. We use DI only to overwrite the + // sanitizer, but not for default one. The default one is pulled in by the Ivy + // instructions as needed. + expect(sanitizer).toBe(null); + } else { + // In VE we always need to have Sanitizer available. + expect(sanitizer).not.toBe(null); + } + })); + it('should throw if no element is found', inject([AsyncTestCompleter], (async: AsyncTestCompleter) => { const logger = new MockConsole(); diff --git a/tools/public_api_guard/core/core.d.ts b/tools/public_api_guard/core/core.d.ts index e6eba1a481..4caac6b04e 100644 --- a/tools/public_api_guard/core/core.d.ts +++ b/tools/public_api_guard/core/core.d.ts @@ -1057,7 +1057,7 @@ export declare function ɵɵstyleMap(styles: { [styleName: string]: any; } | NO_CHANGE | null): void; -export declare function ɵɵstyleProp(prop: string, value: string | number | String | null, suffix?: string | null): void; +export declare function ɵɵstyleProp(prop: string, value: string | number | SafeValue | null, suffix?: string | null): void; export declare function ɵɵstylePropInterpolate1(prop: string, prefix: string, v0: any, suffix: string, valueSuffix?: string | null): TsickleIssue1009; @@ -1314,6 +1314,7 @@ export declare abstract class RootRenderer { export declare abstract class Sanitizer { abstract sanitize(context: SecurityContext, value: {} | string | null): string | null; + static ngInjectableDef: never; } export interface SchemaMetadata {