diff --git a/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts b/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts index 30ff6066b0..19b34ac4a7 100644 --- a/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts +++ b/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts @@ -1475,6 +1475,157 @@ describe('ngtsc behavioral tests', () => { expect(afterCount).toBe(1); }); + describe('sanitization', () => { + it('should generate sanitizers for unsafe attributes in hostBindings fn in Directives', () => { + env.tsconfig(); + env.write(`test.ts`, ` + import {Component, Directive, HostBinding} from '@angular/core'; + + @Directive({ + selector: '[unsafeAttrs]' + }) + class UnsafeAttrsDirective { + @HostBinding('attr.href') + attrHref: string; + + @HostBinding('attr.src') + attrSrc: string; + + @HostBinding('attr.action') + attrAction: string; + + @HostBinding('attr.profile') + attrProfile: string; + + @HostBinding('attr.innerHTML') + attrInnerHTML: string; + + @HostBinding('attr.title') + attrSafeTitle: string; + } + + @Component({ + selector: 'foo', + template: 'Link Title' + }) + class FooCmp {} + `); + + env.driveMain(); + const jsContents = env.getContents('test.js'); + const hostBindingsFn = ` + hostBindings: function UnsafeAttrsDirective_HostBindings(rf, ctx, elIndex) { + if (rf & 1) { + i0.ɵallocHostVars(6); + } + if (rf & 2) { + i0.ɵelementAttribute(elIndex, "href", i0.ɵbind(ctx.attrHref), i0.ɵsanitizeUrlOrResourceUrl); + i0.ɵelementAttribute(elIndex, "src", i0.ɵbind(ctx.attrSrc), i0.ɵsanitizeUrlOrResourceUrl); + i0.ɵelementAttribute(elIndex, "action", i0.ɵbind(ctx.attrAction), i0.ɵsanitizeUrl); + i0.ɵelementAttribute(elIndex, "profile", i0.ɵbind(ctx.attrProfile), i0.ɵsanitizeResourceUrl); + i0.ɵelementAttribute(elIndex, "innerHTML", i0.ɵbind(ctx.attrInnerHTML), i0.ɵsanitizeHtml); + i0.ɵelementAttribute(elIndex, "title", i0.ɵbind(ctx.attrSafeTitle)); + } + } + `; + expect(trim(jsContents)).toContain(trim(hostBindingsFn)); + }); + + it('should generate sanitizers for unsafe properties in hostBindings fn in Directives', () => { + env.tsconfig(); + env.write(`test.ts`, ` + import {Component, Directive, HostBinding} from '@angular/core'; + + @Directive({ + selector: '[unsafeProps]' + }) + class UnsafePropsDirective { + @HostBinding('href') + propHref: string; + + @HostBinding('src') + propSrc: string; + + @HostBinding('action') + propAction: string; + + @HostBinding('profile') + propProfile: string; + + @HostBinding('innerHTML') + propInnerHTML: string; + + @HostBinding('title') + propSafeTitle: string; + } + + @Component({ + selector: 'foo', + template: 'Link Title' + }) + class FooCmp {} + `); + + env.driveMain(); + const jsContents = env.getContents('test.js'); + const hostBindingsFn = ` + hostBindings: function UnsafePropsDirective_HostBindings(rf, ctx, elIndex) { + if (rf & 1) { + i0.ɵallocHostVars(6); + } + if (rf & 2) { + i0.ɵelementProperty(elIndex, "href", i0.ɵbind(ctx.propHref), i0.ɵsanitizeUrlOrResourceUrl, true); + i0.ɵelementProperty(elIndex, "src", i0.ɵbind(ctx.propSrc), i0.ɵsanitizeUrlOrResourceUrl, true); + i0.ɵelementProperty(elIndex, "action", i0.ɵbind(ctx.propAction), i0.ɵsanitizeUrl, true); + i0.ɵelementProperty(elIndex, "profile", i0.ɵbind(ctx.propProfile), i0.ɵsanitizeResourceUrl, true); + i0.ɵelementProperty(elIndex, "innerHTML", i0.ɵbind(ctx.propInnerHTML), i0.ɵsanitizeHtml, true); + i0.ɵelementProperty(elIndex, "title", i0.ɵbind(ctx.propSafeTitle), null, true); + } + } + `; + expect(trim(jsContents)).toContain(trim(hostBindingsFn)); + }); + + it('should not generate sanitizers for URL properties in hostBindings fn in Component', () => { + env.tsconfig(); + env.write(`test.ts`, ` + import {Component} from '@angular/core'; + + @Component({ + selector: 'foo', + template: 'Link Title', + host: { + '[src]': 'srcProp', + '[href]': 'hrefProp', + '[title]': 'titleProp', + '[attr.src]': 'srcAttr', + '[attr.href]': 'hrefAttr', + '[attr.title]': 'titleAttr', + } + }) + class FooCmp {} + `); + + env.driveMain(); + const jsContents = env.getContents('test.js'); + const hostBindingsFn = ` + hostBindings: function FooCmp_HostBindings(rf, ctx, elIndex) { + if (rf & 1) { + i0.ɵallocHostVars(6); + } + if (rf & 2) { + i0.ɵelementProperty(elIndex, "src", i0.ɵbind(ctx.srcProp), null, true); + i0.ɵelementProperty(elIndex, "href", i0.ɵbind(ctx.hrefProp), null, true); + i0.ɵelementProperty(elIndex, "title", i0.ɵbind(ctx.titleProp), null, true); + i0.ɵelementAttribute(elIndex, "src", i0.ɵbind(ctx.srcAttr)); + i0.ɵelementAttribute(elIndex, "href", i0.ɵbind(ctx.hrefAttr)); + i0.ɵelementAttribute(elIndex, "title", i0.ɵbind(ctx.titleAttr)); + } + } + `; + expect(trim(jsContents)).toContain(trim(hostBindingsFn)); + }); + }); }); function expectTokenAtPosition( diff --git a/packages/compiler/src/render3/r3_identifiers.ts b/packages/compiler/src/render3/r3_identifiers.ts index 92c3820425..83139fa7da 100644 --- a/packages/compiler/src/render3/r3_identifiers.ts +++ b/packages/compiler/src/render3/r3_identifiers.ts @@ -213,4 +213,6 @@ export class Identifiers { o.ExternalReference = {name: 'ɵsanitizeResourceUrl', moduleName: CORE}; static sanitizeScript: o.ExternalReference = {name: 'ɵsanitizeScript', moduleName: CORE}; static sanitizeUrl: o.ExternalReference = {name: 'ɵsanitizeUrl', moduleName: CORE}; + static sanitizeUrlOrResourceUrl: + o.ExternalReference = {name: 'ɵsanitizeUrlOrResourceUrl', moduleName: CORE}; } diff --git a/packages/compiler/src/render3/view/compiler.ts b/packages/compiler/src/render3/view/compiler.ts index e881270c1b..fcec424ed4 100644 --- a/packages/compiler/src/render3/view/compiler.ts +++ b/packages/compiler/src/render3/view/compiler.ts @@ -30,7 +30,7 @@ import {prepareSyntheticListenerFunctionName, prepareSyntheticPropertyName, type import {R3ComponentDef, R3ComponentMetadata, R3DirectiveDef, R3DirectiveMetadata, R3QueryMetadata} from './api'; import {StylingBuilder, StylingInstruction} from './styling_builder'; -import {BindingScope, TemplateDefinitionBuilder, ValueConverter, prepareEventListenerParameters, renderFlagCheckIfStmt} from './template'; +import {BindingScope, TemplateDefinitionBuilder, ValueConverter, prepareEventListenerParameters, renderFlagCheckIfStmt, resolveSanitizationFn} from './template'; import {CONTEXT_NAME, DefinitionMap, RENDER_FLAGS, TEMPORARY_NAME, asLiteral, conditionallyCreateMapObjectLiteral, getQueryPredicate, temporaryAllocator} from './util'; const EMPTY_ARRAY: any[] = []; @@ -698,15 +698,45 @@ function createHostBindingsFunction( const value = binding.expression.visit(valueConverter); const bindingExpr = bindingFn(bindingContext, value); - const {bindingName, instruction, extraParams} = getBindingNameAndInstruction(binding); + const {bindingName, instruction, isAttribute} = getBindingNameAndInstruction(binding); + + const securityContexts = + bindingParser + .calcPossibleSecurityContexts(meta.selector || '', bindingName, isAttribute) + .filter(context => context !== core.SecurityContext.NONE); + + let sanitizerFn: o.ExternalExpr|null = null; + if (securityContexts.length) { + if (securityContexts.length === 2 && + securityContexts.indexOf(core.SecurityContext.URL) > -1 && + securityContexts.indexOf(core.SecurityContext.RESOURCE_URL) > -1) { + // Special case for some URL attributes (such as "src" and "href") that may be a part of + // different security contexts. In this case we use special santitization function and + // select the actual sanitizer at runtime based on a tag name that is provided while + // invoking sanitization function. + sanitizerFn = o.importExpr(R3.sanitizeUrlOrResourceUrl); + } else { + sanitizerFn = resolveSanitizationFn(securityContexts[0], isAttribute); + } + } const instructionParams: o.Expression[] = [ elVarExp, o.literal(bindingName), o.importExpr(R3.bind).callFn([bindingExpr.currValExpr]) ]; + if (sanitizerFn) { + instructionParams.push(sanitizerFn); + } + if (!isAttribute) { + if (!sanitizerFn) { + // append `null` in front of `nativeOnly` flag if no sanitizer fn defined + instructionParams.push(o.literal(null)); + } + // host bindings must have nativeOnly prop set to true + instructionParams.push(o.literal(true)); + } updateStatements.push(...bindingExpr.stmts); - updateStatements.push( - o.importExpr(instruction).callFn(instructionParams.concat(extraParams)).toStmt()); + updateStatements.push(o.importExpr(instruction).callFn(instructionParams).toStmt()); } } @@ -777,10 +807,9 @@ function createStylingStmt( } function getBindingNameAndInstruction(binding: ParsedProperty): - {bindingName: string, instruction: o.ExternalReference, extraParams: o.Expression[]} { + {bindingName: string, instruction: o.ExternalReference, isAttribute: boolean} { let bindingName = binding.name; let instruction !: o.ExternalReference; - const extraParams: o.Expression[] = []; // Check to see if this is an attr binding or a property binding const attrMatches = bindingName.match(ATTR_REGEX); @@ -797,13 +826,9 @@ function getBindingNameAndInstruction(binding: ParsedProperty): } else { instruction = R3.elementProperty; } - extraParams.push( - o.literal(null), // TODO: This should be a sanitizer fn (FW-785) - o.literal(true) // host bindings must have nativeOnly prop set to true - ); } - return {bindingName, instruction, extraParams}; + return {bindingName, instruction, isAttribute: !!attrMatches}; } function createHostListeners( diff --git a/packages/compiler/src/render3/view/template.ts b/packages/compiler/src/render3/view/template.ts index e9d085bee3..893b85b3fa 100644 --- a/packages/compiler/src/render3/view/template.ts +++ b/packages/compiler/src/render3/view/template.ts @@ -701,7 +701,8 @@ export class TemplateDefinitionBuilder implements t.Visitor, LocalResolver } } else if (instruction) { const params: any[] = []; - const sanitizationRef = resolveSanitizationFn(input, input.securityContext); + const isAttributeBinding = input.type === BindingType.Attribute; + const sanitizationRef = resolveSanitizationFn(input.securityContext, isAttributeBinding); if (sanitizationRef) params.push(sanitizationRef); // TODO(chuckj): runtime: security context @@ -1571,7 +1572,7 @@ export function makeBindingParser( new Parser(new Lexer()), interpolationConfig, new DomElementSchemaRegistry(), null, []); } -function resolveSanitizationFn(input: t.BoundAttribute, context: core.SecurityContext) { +export function resolveSanitizationFn(context: core.SecurityContext, isAttribute?: boolean) { switch (context) { case core.SecurityContext.HTML: return o.importExpr(R3.sanitizeHtml); @@ -1581,7 +1582,7 @@ function resolveSanitizationFn(input: t.BoundAttribute, context: core.SecurityCo // the compiler does not fill in an instruction for [style.prop?] binding // values because the style algorithm knows internally what props are subject // to sanitization (only [attr.style] values are explicitly sanitized) - return input.type === BindingType.Attribute ? o.importExpr(R3.sanitizeStyle) : null; + return isAttribute ? o.importExpr(R3.sanitizeStyle) : null; case core.SecurityContext.URL: return o.importExpr(R3.sanitizeUrl); case core.SecurityContext.RESOURCE_URL: diff --git a/packages/compiler/src/template_parser/binding_parser.ts b/packages/compiler/src/template_parser/binding_parser.ts index 178fa7c58c..bef79f124b 100644 --- a/packages/compiler/src/template_parser/binding_parser.ts +++ b/packages/compiler/src/template_parser/binding_parser.ts @@ -305,6 +305,12 @@ export class BindingParser { } } + calcPossibleSecurityContexts(selector: string, propName: string, isAttribute: boolean): + SecurityContext[] { + const prop = this._schemaRegistry.getMappedPropName(propName); + return calcPossibleSecurityContexts(this._schemaRegistry, selector, prop, isAttribute); + } + private _parseAnimationEvent( name: string, expression: string, sourceSpan: ParseSourceSpan, targetEvents: ParsedEvent[]) { const matches = splitAtPeriod(name, [name, '']); diff --git a/packages/core/src/core_render3_private_export.ts b/packages/core/src/core_render3_private_export.ts index 27741a3070..38c63da7d0 100644 --- a/packages/core/src/core_render3_private_export.ts +++ b/packages/core/src/core_render3_private_export.ts @@ -151,6 +151,7 @@ export { sanitizeStyle as ɵsanitizeStyle, sanitizeUrl as ɵsanitizeUrl, sanitizeResourceUrl as ɵsanitizeResourceUrl, + sanitizeUrlOrResourceUrl as ɵsanitizeUrlOrResourceUrl, } from './sanitization/sanitization'; export { diff --git a/packages/core/src/render3/instructions.ts b/packages/core/src/render3/instructions.ts index 65d99a86f9..f3ee6ac4c5 100644 --- a/packages/core/src/render3/instructions.ts +++ b/packages/core/src/render3/instructions.ts @@ -976,7 +976,9 @@ export function elementAttribute( element.removeAttribute(name); } else { ngDevMode && ngDevMode.rendererSetAttribute++; - const strValue = sanitizer == null ? stringify(value) : sanitizer(value); + const tNode = getTNode(index, lView); + const strValue = + sanitizer == null ? stringify(value) : sanitizer(value, tNode.tagName || '', name); isProceduralRenderer(renderer) ? renderer.setAttribute(element, name, strValue) : element.setAttribute(name, strValue); } @@ -1059,7 +1061,7 @@ function elementPropertyInternal( const renderer = loadRendererFn ? loadRendererFn(tNode, lView) : lView[RENDERER]; // It is assumed that the sanitizer is only added when the compiler determines that the property // is risky, so sanitization can be done without further checks. - value = sanitizer != null ? (sanitizer(value) as any) : value; + value = sanitizer != null ? (sanitizer(value, tNode.tagName || '', propName) as any) : value; ngDevMode && ngDevMode.rendererSetProperty++; if (isProceduralRenderer(renderer)) { renderer.setProperty(element as RElement, propName, value); diff --git a/packages/core/src/render3/interfaces/sanitization.ts b/packages/core/src/render3/interfaces/sanitization.ts index 24970f93c9..e329c03024 100644 --- a/packages/core/src/render3/interfaces/sanitization.ts +++ b/packages/core/src/render3/interfaces/sanitization.ts @@ -9,4 +9,4 @@ /** * Function used to sanitize the value before writing it into the renderer. */ -export type SanitizerFn = (value: any) => string; +export type SanitizerFn = (value: any, tagName?: string, propName?: string) => string; diff --git a/packages/core/src/render3/jit/environment.ts b/packages/core/src/render3/jit/environment.ts index ffc043661e..6caff12a66 100644 --- a/packages/core/src/render3/jit/environment.ts +++ b/packages/core/src/render3/jit/environment.ts @@ -116,5 +116,6 @@ export const angularCoreEnv: {[name: string]: Function} = { 'ɵdefaultStyleSanitizer': sanitization.defaultStyleSanitizer, 'ɵsanitizeResourceUrl': sanitization.sanitizeResourceUrl, 'ɵsanitizeScript': sanitization.sanitizeScript, - 'ɵsanitizeUrl': sanitization.sanitizeUrl + 'ɵsanitizeUrl': sanitization.sanitizeUrl, + 'ɵsanitizeUrlOrResourceUrl': sanitization.sanitizeUrlOrResourceUrl }; diff --git a/packages/core/src/sanitization/sanitization.ts b/packages/core/src/sanitization/sanitization.ts index 5e03458356..969db77592 100644 --- a/packages/core/src/sanitization/sanitization.ts +++ b/packages/core/src/sanitization/sanitization.ts @@ -132,6 +132,39 @@ export function sanitizeScript(unsafeScript: any): string { throw new Error('unsafe value used in a script context'); } +/** + * Detects which sanitizer to use for URL property, based on tag name and prop name. + * + * The rules are based on the RESOURCE_URL context config from + * `packages/compiler/src/schema/dom_security_schema.ts`. + * If tag and prop names don't match Resource URL schema, use URL sanitizer. + */ +export function getUrlSanitizer(tag: string, prop: string) { + if ((prop === 'src' && (tag === 'embed' || tag === 'frame' || tag === 'iframe' || + tag === 'media' || tag === 'script')) || + (prop === 'href' && (tag === 'base' || tag === 'link'))) { + return sanitizeResourceUrl; + } + return sanitizeUrl; +} + +/** + * Sanitizes URL, selecting sanitizer function based on tag and property names. + * + * This function is used in case we can't define security context at compile time, when only prop + * name is available. This happens when we generate host bindings for Directives/Components. The + * host element is unknown at compile time, so we defer calculation of specific sanitizer to + * runtime. + * + * @param unsafeUrl untrusted `url`, typically from the user. + * @param tag target element tag name. + * @param prop name of the property that contains the value. + * @returns `url` string which is safe to bind. + */ +export function sanitizeUrlOrResourceUrl(unsafeUrl: any, tag: string, prop: string): any { + return getUrlSanitizer(tag, prop)(unsafeUrl); +} + /** * The default style sanitizer will handle sanitization for style properties by * sanitizing any CSS property that can include a `url` value (usually image-based properties) diff --git a/packages/core/test/linker/security_integration_spec.ts b/packages/core/test/linker/security_integration_spec.ts index 9fcab8d774..8baa99918c 100644 --- a/packages/core/test/linker/security_integration_spec.ts +++ b/packages/core/test/linker/security_integration_spec.ts @@ -171,39 +171,37 @@ function declareTests(config?: {useJit: boolean}) { checkEscapeOfHrefProperty(fixture, true); }); - fixmeIvy('FW-785: Host bindings are not sanitised') - .it('should escape unsafe properties if they are used in host bindings', () => { - @Directive({selector: '[dirHref]'}) - class HrefDirective { - // TODO(issue/24571): remove '!'. - @HostBinding('href') @Input() - dirHref !: string; - } + it('should escape unsafe properties if they are used in host bindings', () => { + @Directive({selector: '[dirHref]'}) + class HrefDirective { + // TODO(issue/24571): remove '!'. + @HostBinding('href') @Input() + dirHref !: string; + } - const template = `Link Title`; - TestBed.configureTestingModule({declarations: [HrefDirective]}); - TestBed.overrideComponent(SecuredComponent, {set: {template}}); - const fixture = TestBed.createComponent(SecuredComponent); + const template = `Link Title`; + TestBed.configureTestingModule({declarations: [HrefDirective]}); + TestBed.overrideComponent(SecuredComponent, {set: {template}}); + const fixture = TestBed.createComponent(SecuredComponent); - checkEscapeOfHrefProperty(fixture, false); - }); + checkEscapeOfHrefProperty(fixture, false); + }); - fixmeIvy('FW-785: Host bindings are not sanitised') - .it('should escape unsafe attributes if they are used in host bindings', () => { - @Directive({selector: '[dirHref]'}) - class HrefDirective { - // TODO(issue/24571): remove '!'. - @HostBinding('attr.href') @Input() - dirHref !: string; - } + it('should escape unsafe attributes if they are used in host bindings', () => { + @Directive({selector: '[dirHref]'}) + class HrefDirective { + // TODO(issue/24571): remove '!'. + @HostBinding('attr.href') @Input() + dirHref !: string; + } - const template = `Link Title`; - TestBed.configureTestingModule({declarations: [HrefDirective]}); - TestBed.overrideComponent(SecuredComponent, {set: {template}}); - const fixture = TestBed.createComponent(SecuredComponent); + const template = `Link Title`; + TestBed.configureTestingModule({declarations: [HrefDirective]}); + TestBed.overrideComponent(SecuredComponent, {set: {template}}); + const fixture = TestBed.createComponent(SecuredComponent); - checkEscapeOfHrefProperty(fixture, true); - }); + checkEscapeOfHrefProperty(fixture, true); + }); it('should escape unsafe style values', () => { const template = `
Text
`; diff --git a/packages/core/test/render3/host_binding_spec.ts b/packages/core/test/render3/host_binding_spec.ts index f3a7abf829..424456d816 100644 --- a/packages/core/test/render3/host_binding_spec.ts +++ b/packages/core/test/render3/host_binding_spec.ts @@ -9,10 +9,12 @@ import {ElementRef, QueryList} from '@angular/core'; import {AttributeMarker, defineComponent, template, defineDirective, InheritDefinitionFeature, ProvidersFeature, NgOnChangesFeature} from '../../src/render3/index'; -import {allocHostVars, bind, directiveInject, element, elementEnd, elementProperty, elementStyleProp, elementStyling, elementStylingApply, elementStart, listener, load, text, textBinding, loadQueryList, registerContentQuery, elementHostAttrs} from '../../src/render3/instructions'; +import {allocHostVars, bind, directiveInject, element, elementAttribute, elementEnd, elementProperty, elementStyleProp, elementStyling, elementStylingApply, elementStart, listener, load, text, textBinding, loadQueryList, registerContentQuery, elementHostAttrs} from '../../src/render3/instructions'; import {query, queryRefresh} from '../../src/render3/query'; import {RenderFlags} from '../../src/render3/interfaces/definition'; import {pureFunction1, pureFunction2} from '../../src/render3/pure_function'; +import {sanitizeUrl, sanitizeUrlOrResourceUrl, sanitizeHtml} from '../../src/sanitization/sanitization'; +import {bypassSanitizationTrustHtml, bypassSanitizationTrustResourceUrl, bypassSanitizationTrustUrl} from '../../src/sanitization/bypass'; import {ComponentFixture, TemplateFixture, createComponent, createDirective} from './render_util'; import {NgForOf} from './common_with_def'; @@ -1164,4 +1166,60 @@ describe('host bindings', () => { expect(hostBindingEl.className).toEqual('mat-toolbar'); }); }); + + describe('sanitization', () => { + function verify( + tag: string, prop: string, value: any, expectedSanitizedValue: any, sanitizeFn: any, + bypassFn: any, isAttribute: boolean = true) { + it('should sanitize potentially unsafe properties and attributes', () => { + let hostBindingDir: UnsafeUrlHostBindingDir; + class UnsafeUrlHostBindingDir { + // val: any = value; + static ngDirectiveDef = defineDirective({ + type: UnsafeUrlHostBindingDir, + selectors: [['', 'unsafeUrlHostBindingDir', '']], + factory: () => hostBindingDir = new UnsafeUrlHostBindingDir(), + hostBindings: (rf: RenderFlags, ctx: any, elementIndex: number) => { + if (rf & RenderFlags.Create) { + allocHostVars(1); + } + if (rf & RenderFlags.Update) { + const fn = isAttribute ? elementAttribute : elementProperty; + (fn as any)(elementIndex, prop, bind(ctx[prop]), sanitizeFn, true); + } + } + }); + } + + const fixture = new TemplateFixture(() => { + element(0, tag, ['unsafeUrlHostBindingDir', '']); + }, () => {}, 1, 0, [UnsafeUrlHostBindingDir]); + + const el = fixture.hostElement.querySelector(tag) !; + const current = () => isAttribute ? el.getAttribute(prop) : (el as any)[prop]; + + (hostBindingDir !as any)[prop] = value; + fixture.update(); + expect(current()).toEqual(expectedSanitizedValue); + + (hostBindingDir !as any)[prop] = bypassFn(value); + fixture.update(); + expect(current()).toEqual(value); + }); + } + + verify( + 'a', 'href', 'javascript:alert(1)', 'unsafe:javascript:alert(1)', sanitizeUrlOrResourceUrl, + bypassSanitizationTrustUrl); + verify( + 'script', 'src', bypassSanitizationTrustResourceUrl('javascript:alert(2)'), + 'javascript:alert(2)', sanitizeUrlOrResourceUrl, bypassSanitizationTrustResourceUrl); + verify( + 'blockquote', 'cite', 'javascript:alert(3)', 'unsafe:javascript:alert(3)', sanitizeUrl, + bypassSanitizationTrustUrl); + verify( + 'b', 'innerHTML', '', + '', sanitizeHtml, bypassSanitizationTrustHtml, + /* isAttribute */ false); + }); }); diff --git a/packages/core/test/render3/integration_spec.ts b/packages/core/test/render3/integration_spec.ts index 0a4c784e41..4acfc3ce65 100644 --- a/packages/core/test/render3/integration_spec.ts +++ b/packages/core/test/render3/integration_spec.ts @@ -2878,6 +2878,58 @@ describe('render3 integration test', () => { expect(anchor.getAttribute('href')).toEqual('http://foo'); }); + + it('should sanitize HostBindings data using provided sanitization interface', () => { + let hostBindingDir: UnsafeUrlHostBindingDir; + class UnsafeUrlHostBindingDir { + // @HostBinding() + cite: any = 'http://cite-dir-value'; + + static ngDirectiveDef = defineDirective({ + type: UnsafeUrlHostBindingDir, + selectors: [['', 'unsafeUrlHostBindingDir', '']], + factory: () => hostBindingDir = new UnsafeUrlHostBindingDir(), + hostBindings: (rf: RenderFlags, ctx: any, elementIndex: number) => { + if (rf & RenderFlags.Create) { + allocHostVars(1); + } + if (rf & RenderFlags.Update) { + elementProperty(elementIndex, 'cite', bind(ctx.cite), sanitizeUrl, true); + } + } + }); + } + + class SimpleComp { + static ngComponentDef = defineComponent({ + type: SimpleComp, + selectors: [['sanitize-this']], + factory: () => new SimpleComp(), + consts: 1, + vars: 0, + template: (rf: RenderFlags, ctx: SimpleComp) => { + if (rf & RenderFlags.Create) { + element(0, 'blockquote', ['unsafeUrlHostBindingDir', '']); + } + }, + directives: [UnsafeUrlHostBindingDir] + }); + } + + const sanitizer = new LocalSanitizer((value) => 'http://bar'); + + const fixture = new ComponentFixture(SimpleComp, {sanitizer}); + hostBindingDir !.cite = 'http://foo'; + fixture.update(); + + const anchor = fixture.hostElement.querySelector('blockquote') !; + expect(anchor.getAttribute('cite')).toEqual('http://bar'); + + hostBindingDir !.cite = sanitizer.bypassSecurityTrustUrl('http://foo'); + fixture.update(); + + expect(anchor.getAttribute('cite')).toEqual('http://foo'); + }); }); }); diff --git a/packages/core/test/sanitization/sanatization_spec.ts b/packages/core/test/sanitization/sanatization_spec.ts index db61da433d..5d538b6305 100644 --- a/packages/core/test/sanitization/sanatization_spec.ts +++ b/packages/core/test/sanitization/sanatization_spec.ts @@ -7,10 +7,12 @@ * found in the LICENSE file at https://angular.io/license */ +import {SECURITY_SCHEMA} from '@angular/compiler/src/schema/dom_security_schema'; import {setTNodeAndViewData} from '@angular/core/src/render3/state'; import {bypassSanitizationTrustHtml, bypassSanitizationTrustResourceUrl, bypassSanitizationTrustScript, bypassSanitizationTrustStyle, bypassSanitizationTrustUrl} from '../../src/sanitization/bypass'; -import {sanitizeHtml, sanitizeResourceUrl, sanitizeScript, sanitizeStyle, sanitizeUrl} from '../../src/sanitization/sanitization'; +import {getUrlSanitizer, sanitizeHtml, sanitizeResourceUrl, sanitizeScript, sanitizeStyle, sanitizeUrl, sanitizeUrlOrResourceUrl} from '../../src/sanitization/sanitization'; +import {SecurityContext} from '../../src/sanitization/security'; describe('sanitization', () => { beforeEach(() => setTNodeAndViewData(null !, [] as any)); @@ -69,4 +71,54 @@ describe('sanitization', () => { expect(() => sanitizeScript(bypassSanitizationTrustHtml('true'))).toThrowError(ERROR); expect(sanitizeScript(bypassSanitizationTrustScript('true'))).toEqual('true'); }); + + it('should select correct sanitizer for URL props', () => { + // making sure security schema we have on compiler side is in sync with the `getUrlSanitizer` + // runtime function definition + const schema = SECURITY_SCHEMA(); + const contextsByProp: Map> = new Map(); + const sanitizerNameByContext: Map = new Map([ + [SecurityContext.URL, 'sanitizeUrl'], [SecurityContext.RESOURCE_URL, 'sanitizeResourceUrl'] + ]); + Object.keys(schema).forEach(key => { + const context = schema[key]; + if (context === SecurityContext.URL || SecurityContext.RESOURCE_URL) { + const [tag, prop] = key.split('|'); + const contexts = contextsByProp.get(prop) || new Set(); + contexts.add(context); + contextsByProp.set(prop, contexts); + // check only in case a prop can be a part of both URL contexts + if (contexts.size === 2) { + expect(getUrlSanitizer(tag, prop).name).toEqual(sanitizerNameByContext.get(context) !); + } + } + }); + }); + + it('should sanitize resourceUrls via sanitizeUrlOrResourceUrl', () => { + const ERROR = 'unsafe value used in a resource URL context (see http://g.co/ng/security#xss)'; + expect(() => sanitizeUrlOrResourceUrl('http://server', 'iframe', 'src')).toThrowError(ERROR); + expect(() => sanitizeUrlOrResourceUrl('javascript:true', 'iframe', 'src')).toThrowError(ERROR); + expect( + () => sanitizeUrlOrResourceUrl( + bypassSanitizationTrustHtml('javascript:true'), 'iframe', 'src')) + .toThrowError(ERROR); + expect(sanitizeUrlOrResourceUrl( + bypassSanitizationTrustResourceUrl('javascript:true'), 'iframe', 'src')) + .toEqual('javascript:true'); + }); + + it('should sanitize urls via sanitizeUrlOrResourceUrl', () => { + expect(sanitizeUrlOrResourceUrl('http://server', 'a', 'href')).toEqual('http://server'); + expect(sanitizeUrlOrResourceUrl(new Wrap('http://server'), 'a', 'href')) + .toEqual('http://server'); + expect(sanitizeUrlOrResourceUrl('javascript:true', 'a', 'href')) + .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(bypassSanitizationTrustUrl('javascript:true'), 'a', 'href')) + .toEqual('javascript:true'); + }); });