Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(ivy): sanitization for Host Bindings #27939

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
151 changes: 151 additions & 0 deletions packages/compiler-cli/test/ngtsc/ngtsc_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1281,4 +1281,155 @@ 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')
AndrewKushnir marked this conversation as resolved.
Show resolved Hide resolved
attrInnerHTML: string;

@HostBinding('attr.title')
attrSafeTitle: string;
}

@Component({
selector: 'foo',
template: '<a [unsafeAttrs]="ctxProp">Link Title</a>'
})
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: '<a [unsafeProps]="ctxProp">Link Title</a>'
})
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: '<a href="example.com">Link Title</a>',
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));
});
});
});
2 changes: 2 additions & 0 deletions packages/compiler/src/render3/r3_identifiers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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};
}
47 changes: 36 additions & 11 deletions packages/compiler/src/render3/view/compiler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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[] = [];
Expand Down Expand Up @@ -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());
}
}

Expand Down Expand Up @@ -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);
Expand All @@ -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(
Expand Down
7 changes: 4 additions & 3 deletions packages/compiler/src/render3/view/template.ts
Original file line number Diff line number Diff line change
Expand Up @@ -701,7 +701,8 @@ export class TemplateDefinitionBuilder implements t.Visitor<void>, 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
Expand Down Expand Up @@ -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);
Expand All @@ -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:
Expand Down
6 changes: 6 additions & 0 deletions packages/compiler/src/template_parser/binding_parser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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, '']);
Expand Down
1 change: 1 addition & 0 deletions packages/core/src/core_render3_private_export.ts
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,7 @@ export {
sanitizeStyle as ɵsanitizeStyle,
sanitizeUrl as ɵsanitizeUrl,
sanitizeResourceUrl as ɵsanitizeResourceUrl,
sanitizeUrlOrResourceUrl as ɵsanitizeUrlOrResourceUrl,
} from './sanitization/sanitization';

export {
Expand Down
6 changes: 4 additions & 2 deletions packages/core/src/render3/instructions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down Expand Up @@ -1059,7 +1061,7 @@ function elementPropertyInternal<T>(
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);
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/render3/interfaces/sanitization.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
3 changes: 2 additions & 1 deletion packages/core/src/render3/jit/environment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
};
33 changes: 33 additions & 0 deletions packages/core/src/sanitization/sanitization.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down