refactor(compiler): Generate trusted const values for extracted attrs (#53473)

Use the DomElementSchemaRegistry to determine the correct security
context for static attributes, and pass it along during ingestion. Then
during the resolve sanitizers phase, use the security context to
determine if a trusted value function is needed

PR Close #53473
This commit is contained in:
Miles Malerba 2023-12-09 10:41:13 -08:00 committed by Alex Rickabaugh
parent 1fc5442947
commit 44bf6d6dec
11 changed files with 164 additions and 41 deletions

View file

@ -253,8 +253,7 @@
{
"failureMessage": "Incorrect generated template."
}
],
"skipForTemplatePipeline": true
]
}
]
}
}

View file

@ -606,7 +606,7 @@ function createHostBindingsFunction(
events: eventBindings,
attributes: hostBindingsMetadata.attributes,
},
bindingParser, constantPool);
constantPool);
transform(hostJob, CompilationJobKind.Host);
definitionMap.set('hostAttrs', hostJob.root.attributes);

View file

@ -350,6 +350,11 @@ export enum ExpressionKind {
*/
SanitizerExpr,
/**
* An expression representing a function to create trusted values.
*/
TrustedValueFnExpr,
/**
* An expression that will cause a literal slot index to be emitted.
*/
@ -429,6 +434,14 @@ export enum SanitizerFn {
IframeAttribute,
}
/**
* Represents functions used to create trusted values.
*/
export enum TrustedValueFn {
Html,
ResourceUrl,
}
/**
* Enumeration of the different kinds of `@defer` secondary blocks.
*/

View file

@ -10,7 +10,7 @@ import * as o from '../../../../output/output_ast';
import type {ParseSourceSpan} from '../../../../parse_util';
import * as t from '../../../../render3/r3_ast';
import {DerivedRepeaterVarIdentity, ExpressionKind, OpKind, SanitizerFn} from './enums';
import {DerivedRepeaterVarIdentity, ExpressionKind, OpKind, SanitizerFn, TrustedValueFn} from './enums';
import {SlotHandle} from './handle';
import type {XrefId} from './operations';
import type {CreateOp} from './ops/create';
@ -20,11 +20,12 @@ import {ConsumesVarsTrait, UsesVarOffset, UsesVarOffsetTrait} from './traits';
/**
* An `o.Expression` subtype representing a logical expression in the intermediate representation.
*/
export type Expression = LexicalReadExpr|ReferenceExpr|ContextExpr|NextContextExpr|
GetCurrentViewExpr|RestoreViewExpr|ResetViewExpr|ReadVariableExpr|PureFunctionExpr|
PureFunctionParameterExpr|PipeBindingExpr|PipeBindingVariadicExpr|SafePropertyReadExpr|
SafeKeyedReadExpr|SafeInvokeFunctionExpr|EmptyExpr|AssignTemporaryExpr|ReadTemporaryExpr|
SanitizerExpr|SlotLiteralExpr|ConditionalCaseExpr|DerivedRepeaterVarExpr|ConstCollectedExpr;
export type Expression =
LexicalReadExpr|ReferenceExpr|ContextExpr|NextContextExpr|GetCurrentViewExpr|RestoreViewExpr|
ResetViewExpr|ReadVariableExpr|PureFunctionExpr|PureFunctionParameterExpr|PipeBindingExpr|
PipeBindingVariadicExpr|SafePropertyReadExpr|SafeKeyedReadExpr|SafeInvokeFunctionExpr|EmptyExpr|
AssignTemporaryExpr|ReadTemporaryExpr|SanitizerExpr|TrustedValueFnExpr|SlotLiteralExpr|
ConditionalCaseExpr|DerivedRepeaterVarExpr|ConstCollectedExpr;
/**
* Transformer type which converts expressions into general `o.Expression`s (which may be an
@ -756,6 +757,30 @@ export class SanitizerExpr extends ExpressionBase {
override transformInternalExpressions(): void {}
}
export class TrustedValueFnExpr extends ExpressionBase {
override readonly kind = ExpressionKind.TrustedValueFnExpr;
constructor(public fn: TrustedValueFn) {
super();
}
override visitExpression(visitor: o.ExpressionVisitor, context: any): any {}
override isEquivalent(e: Expression): boolean {
return e instanceof TrustedValueFnExpr && e.fn === this.fn;
}
override isConstant() {
return true;
}
override clone(): TrustedValueFnExpr {
return new TrustedValueFnExpr(this.fn);
}
override transformInternalExpressions(): void {}
}
export class SlotLiteralExpr extends ExpressionBase {
override readonly kind = ExpressionKind.SlotLiteralExpr;
@ -968,6 +993,8 @@ export function transformExpressionsInOp(
case OpKind.ExtractedAttribute:
op.expression =
op.expression && transformExpressionsInExpression(op.expression, transform, flags);
op.trustedValueFn = op.trustedValueFn &&
transformExpressionsInExpression(op.trustedValueFn, transform, flags);
break;
case OpKind.RepeaterCreate:
op.track = transformExpressionsInExpression(op.track, transform, flags);
@ -1087,6 +1114,10 @@ export function transformExpressionsInExpression(
}
} else if (expr instanceof o.NotExpr) {
expr.condition = transformExpressionsInExpression(expr.condition, transform, flags);
} else if (expr instanceof o.TaggedTemplateExpr) {
expr.tag = transformExpressionsInExpression(expr.tag, transform, flags);
expr.template.expressions =
expr.template.expressions.map(e => transformExpressionsInExpression(e, transform, flags));
} else if (
expr instanceof o.ReadVarExpr || expr instanceof o.ExternalExpr ||
expr instanceof o.LiteralExpr) {

View file

@ -6,6 +6,7 @@
* found in the LICENSE file at https://angular.io/license
*/
import {SecurityContext} from '../../../../../core';
import * as i18n from '../../../../../i18n/i18n_ast';
import * as o from '../../../../../output/output_ast';
import {ParseSourceSpan} from '../../../../../parse_util';
@ -660,6 +661,16 @@ export interface ExtractedAttributeOp extends Op<CreateOp> {
*/
i18nContext: XrefId|null;
/**
* The security context of the binding.
*/
securityContext: SecurityContext;
/**
* The trusted value function for this property.
*/
trustedValueFn: o.Expression|null;
i18nMessage: i18n.Message|null;
}
@ -668,7 +679,8 @@ export interface ExtractedAttributeOp extends Op<CreateOp> {
*/
export function createExtractedAttributeOp(
target: XrefId, bindingKind: BindingKind, name: string, expression: o.Expression|null,
i18nContext: XrefId|null, i18nMessage: i18n.Message|null): ExtractedAttributeOp {
i18nContext: XrefId|null, i18nMessage: i18n.Message|null,
securityContext: SecurityContext): ExtractedAttributeOp {
return {
kind: OpKind.ExtractedAttribute,
target,
@ -677,6 +689,8 @@ export function createExtractedAttributeOp(
expression,
i18nContext,
i18nMessage,
securityContext,
trustedValueFn: null,
...NEW_OP,
};
}

View file

@ -16,7 +16,7 @@ import {ParseSourceSpan} from '../../../parse_util';
import * as t from '../../../render3/r3_ast';
import {R3DeferBlockMetadata} from '../../../render3/view/api';
import {icuFromI18nMessage, isSingleI18nIcu} from '../../../render3/view/i18n/util';
import {BindingParser} from '../../../template_parser/binding_parser';
import {DomElementSchemaRegistry} from '../../../schema/dom_element_schema_registry';
import * as ir from '../ir';
import {CompilationUnit, ComponentCompilationJob, HostBindingCompilationJob, type CompilationJob, type ViewCompilationUnit} from './compilation';
@ -24,6 +24,12 @@ import {BINARY_OPERATORS, namespaceForKey, prefixWithNamespace} from './conversi
const compatibilityMode = ir.CompatibilityMode.TemplateDefinitionBuilder;
// Schema containing DOM elements and their properties.
const domSchema = new DomElementSchemaRegistry();
// Tag name of the `ng-template` element.
const NG_TEMPLATE_TAG_NAME = 'ng-template';
/**
* Process a template AST and convert it into a `ComponentCompilation` in the intermediate
* representation.
@ -52,8 +58,7 @@ export interface HostBindingInput {
* representation.
*/
export function ingestHostBinding(
input: HostBindingInput, bindingParser: BindingParser,
constantPool: ConstantPool): HostBindingCompilationJob {
input: HostBindingInput, constantPool: ConstantPool): HostBindingCompilationJob {
const job = new HostBindingCompilationJob(input.componentName, constantPool, compatibilityMode);
for (const property of input.properties ?? []) {
ingestHostProperty(job, property, false);
@ -250,9 +255,10 @@ function ingestContent(unit: ViewCompilationUnit, content: t.Content): void {
const op = ir.createProjectionOp(
unit.job.allocateXrefId(), content.selector, content.i18n, attrs, content.sourceSpan);
for (const attr of content.attributes) {
const securityContext = domSchema.securityContext(content.name, attr.name, true);
unit.update.push(ir.createBindingOp(
op.xref, ir.BindingKind.Attribute, attr.name, o.literal(attr.value), null,
SecurityContext.NONE, true, false, null, asMessage(attr.i18n), attr.sourceSpan));
op.xref, ir.BindingKind.Attribute, attr.name, o.literal(attr.value), null, securityContext,
true, false, null, asMessage(attr.i18n), attr.sourceSpan));
}
unit.create.push(op);
}
@ -790,7 +796,7 @@ const BINDING_KINDS = new Map<e.BindingType, ir.BindingKind>([
* | `<ng-template *ngIf>` (structural) | null |
*/
function isPlainTemplate(tmpl: t.Template) {
return splitNsName(tmpl.tagName ?? '')[1] === 'ng-template';
return splitNsName(tmpl.tagName ?? '')[1] === NG_TEMPLATE_TAG_NAME;
}
/**
@ -816,10 +822,11 @@ function ingestElementBindings(
for (const attr of element.attributes) {
// Attribute literal bindings, such as `attr.foo="bar"`.
const securityContext = domSchema.securityContext(element.name, attr.name, true);
bindings.push(ir.createBindingOp(
op.xref, ir.BindingKind.Attribute, attr.name,
convertAstWithInterpolation(unit.job, attr.value, attr.i18n), null, SecurityContext.NONE,
true, false, null, asMessage(attr.i18n), attr.sourceSpan));
convertAstWithInterpolation(unit.job, attr.value, attr.i18n), null, securityContext, true,
false, null, asMessage(attr.i18n), attr.sourceSpan));
}
for (const input of element.inputs) {
@ -865,8 +872,9 @@ function ingestTemplateBindings(
for (const attr of template.templateAttrs) {
if (attr instanceof t.TextAttribute) {
const securityContext = domSchema.securityContext(NG_TEMPLATE_TAG_NAME, attr.name, true);
bindings.push(createTemplateBinding(
unit, op.xref, e.BindingType.Attribute, attr.name, attr.value, null, SecurityContext.NONE,
unit, op.xref, e.BindingType.Attribute, attr.name, attr.value, null, securityContext,
true, templateKind, asMessage(attr.i18n), attr.sourceSpan));
} else {
bindings.push(createTemplateBinding(
@ -877,9 +885,10 @@ function ingestTemplateBindings(
for (const attr of template.attributes) {
// Attribute literal bindings, such as `attr.foo="bar"`.
const securityContext = domSchema.securityContext(NG_TEMPLATE_TAG_NAME, attr.name, true);
bindings.push(createTemplateBinding(
unit, op.xref, e.BindingType.Attribute, attr.name, attr.value, null, SecurityContext.NONE,
false, templateKind, asMessage(attr.i18n), attr.sourceSpan));
unit, op.xref, e.BindingType.Attribute, attr.name, attr.value, null, securityContext, false,
templateKind, asMessage(attr.i18n), attr.sourceSpan));
}
for (const input of template.inputs) {
@ -907,8 +916,9 @@ function ingestTemplateBindings(
if (templateKind === ir.TemplateKind.Structural &&
output.type !== e.ParsedEventType.Animation) {
// Animation bindings are excluded from the structural template's const array.
const securityContext = domSchema.securityContext(NG_TEMPLATE_TAG_NAME, output.name, true);
unit.create.push(ir.createExtractedAttributeOp(
op.xref, ir.BindingKind.Property, output.name, null, null, null));
op.xref, ir.BindingKind.Property, output.name, null, null, null, securityContext));
}
}
@ -965,7 +975,7 @@ function createTemplateBinding(
// the ng-template's consts (e.g. for the purposes of directive matching). However, we should
// not generate an update instruction for it.
return ir.createExtractedAttributeOp(
xref, ir.BindingKind.Property, name, null, null, i18nMessage);
xref, ir.BindingKind.Property, name, null, null, i18nMessage, securityContext);
}
if (!isTextBinding && (type === e.BindingType.Attribute || type === e.BindingType.Animation)) {
@ -1123,15 +1133,16 @@ function ingestControlFlowInsertionPoint(
// and they can be used in directive matching (in the case of `Template.templateAttrs`).
if (root !== null) {
for (const attr of root.attributes) {
const securityContext = domSchema.securityContext(NG_TEMPLATE_TAG_NAME, attr.name, true);
unit.update.push(ir.createBindingOp(
xref, ir.BindingKind.Attribute, attr.name, o.literal(attr.value), null,
SecurityContext.NONE, true, false, null, asMessage(attr.i18n), attr.sourceSpan));
xref, ir.BindingKind.Attribute, attr.name, o.literal(attr.value), null, securityContext,
true, false, null, asMessage(attr.i18n), attr.sourceSpan));
}
const tagName = root instanceof t.Element ? root.name : root.tagName;
// Don't pass along `ng-template` tag name since it enables directive matching.
return tagName === 'ng-template' ? null : tagName;
return tagName === NG_TEMPLATE_TAG_NAME ? null : tagName;
}
return null;

View file

@ -7,6 +7,7 @@
*/
import {SecurityContext} from '../../../../core';
import * as ir from '../../ir';
import {CompilationJobKind, type CompilationJob, type CompilationUnit} from '../compilation';
import {createOpXrefMap} from '../util/elements';
@ -38,7 +39,9 @@ export function extractAttributes(job: CompilationJob): void {
ir.OpList.insertBefore<ir.CreateOp>(
// Deliberaly null i18nMessage value
ir.createExtractedAttributeOp(op.target, bindingKind, op.name, null, null, null),
ir.createExtractedAttributeOp(
op.target, bindingKind, op.name, /* expression */ null, /* i18nContext */ null,
/* i18nMessage */ null, op.securityContext),
lookupElement(elements, op.target));
}
break;
@ -53,14 +56,18 @@ export function extractAttributes(job: CompilationJob): void {
op.expression instanceof ir.EmptyExpr) {
ir.OpList.insertBefore<ir.CreateOp>(
ir.createExtractedAttributeOp(
op.target, ir.BindingKind.Property, op.name, null, null, null),
op.target, ir.BindingKind.Property, op.name, /* expression */ null,
/* i18nContext */ null,
/* i18nMessage */ null, SecurityContext.STYLE),
lookupElement(elements, op.target));
}
break;
case ir.OpKind.Listener:
if (!op.isAnimationListener) {
const extractedAttributeOp = ir.createExtractedAttributeOp(
op.target, ir.BindingKind.Property, op.name, null, null, null);
op.target, ir.BindingKind.Property, op.name, /* expression */ null,
/* i18nContext */ null,
/* i18nMessage */ null, SecurityContext.NONE);
if (job.kind === CompilationJobKind.Host) {
// This attribute will apply to the enclosing host binding compilation unit, so order
// doesn't matter.
@ -120,7 +127,7 @@ function extractAttributeOp(
const extractedAttributeOp = ir.createExtractedAttributeOp(
op.target,
op.isStructuralTemplateAttribute ? ir.BindingKind.Template : ir.BindingKind.Attribute,
op.name, op.expression, op.i18nContext, op.i18nMessage);
op.name, op.expression, op.i18nContext, op.i18nMessage, op.securityContext);
if (unit.job.kind === CompilationJobKind.Host) {
// This attribute will apply to the enclosing host binding compilation unit, so order doesn't
// matter.

View file

@ -6,10 +6,12 @@
* found in the LICENSE file at https://angular.io/license
*/
import * as core from '../../../../core';
import {splitNsName} from '../../../../ml_parser/tags';
import * as o from '../../../../output/output_ast';
import * as ir from '../../ir';
import {ComponentCompilationJob, HostBindingCompilationJob, type CompilationJob} from '../compilation';
import {literalOrArrayLiteral} from '../conversion';
@ -25,7 +27,7 @@ export function collectElementConsts(job: CompilationJob): void {
if (op.kind === ir.OpKind.ExtractedAttribute) {
const attributes = allElementAttributes.get(op.target) || new ElementAttributes();
allElementAttributes.set(op.target, attributes);
attributes.add(op.bindingKind, op.name, op.expression);
attributes.add(op.bindingKind, op.name, op.expression, op.trustedValueFn);
ir.OpList.remove<ir.CreateOp>(op);
}
}
@ -100,7 +102,8 @@ class ElementAttributes {
return this.byKind.get(ir.BindingKind.I18n) ?? FLYWEIGHT_ARRAY;
}
add(kind: ir.BindingKind, name: string, value: o.Expression|null): void {
add(kind: ir.BindingKind, name: string, value: o.Expression|null,
trustedValueFn: o.Expression|null): void {
if (this.known.has(name)) {
return;
}
@ -123,7 +126,16 @@ class ElementAttributes {
if (value === null) {
throw Error('Attribute, i18n attribute, & style element attributes must have a value');
}
array.push(value);
if (trustedValueFn !== null) {
if (!ir.isStringLiteral(value)) {
throw Error('AssertionError: extracted attribute value should be string literal');
}
array.push(o.taggedTemplate(
trustedValueFn, new o.TemplateLiteral([new o.TemplateLiteralElement(value.value)], []),
undefined, value.sourceSpan));
} else {
array.push(value);
}
}
}

View file

@ -6,9 +6,11 @@
* found in the LICENSE file at https://angular.io/license
*/
import {SecurityContext} from '../../../../core';
import * as o from '../../../../output/output_ast';
import {parse as parseStyle} from '../../../../render3/view/style_parser';
import * as ir from '../../ir';
import type {CompilationJob} from '../compilation';
/**
@ -26,7 +28,7 @@ export function parseExtractedStyles(job: CompilationJob) {
ir.OpList.insertBefore<ir.CreateOp>(
ir.createExtractedAttributeOp(
op.target, ir.BindingKind.StyleProperty, parsedStyles[i],
o.literal(parsedStyles[i + 1]), null, null),
o.literal(parsedStyles[i + 1]), null, null, SecurityContext.STYLE),
op);
}
ir.OpList.remove<ir.CreateOp>(op);
@ -35,7 +37,8 @@ export function parseExtractedStyles(job: CompilationJob) {
for (const parsedClass of parsedClasses) {
ir.OpList.insertBefore<ir.CreateOp>(
ir.createExtractedAttributeOp(
op.target, ir.BindingKind.ClassName, parsedClass, null, null, null),
op.target, ir.BindingKind.ClassName, parsedClass, null, null, null,
SecurityContext.NONE),
op);
}
ir.OpList.remove<ir.CreateOp>(op);

View file

@ -9,7 +9,7 @@
import * as o from '../../../../output/output_ast';
import {Identifiers} from '../../../../render3/r3_identifiers';
import * as ir from '../../ir';
import {ViewCompilationUnit, type CompilationJob, type CompilationUnit} from '../compilation';
import {ComponentCompilationJob, ViewCompilationUnit, type CompilationJob, type CompilationUnit} from '../compilation';
import * as ng from '../instruction';
/**
@ -23,6 +23,11 @@ const sanitizerIdentifierMap = new Map<ir.SanitizerFn, o.ExternalReference>([
[ir.SanitizerFn.Style, Identifiers.sanitizeStyle], [ir.SanitizerFn.Url, Identifiers.sanitizeUrl]
]);
const trustedValueFnIdentifierMap = new Map<ir.TrustedValueFn, o.ExternalReference>([
[ir.TrustedValueFn.Html, Identifiers.trustConstantHtml],
[ir.TrustedValueFn.ResourceUrl, Identifiers.trustConstantResourceUrl],
]);
/**
* Compiles semantic operations across all views and generates output `o.Statement`s with actual
* runtime calls in their place.
@ -36,6 +41,16 @@ export function reify(job: CompilationJob): void {
reifyCreateOperations(unit, unit.create);
reifyUpdateOperations(unit, unit.update);
}
if (job instanceof ComponentCompilationJob) {
reifyConsts(job);
}
}
function reifyConsts(job: ComponentCompilationJob) {
for (let i = 0; i < job.consts.length; i++) {
job.consts[i] = ir.transformExpressionsInExpression(
job.consts[i], reifyIrExpression, ir.VisitorContextFlag.None);
}
}
function reifyCreateOperations(unit: CompilationUnit, ops: ir.OpList<ir.CreateOp>): void {
@ -417,6 +432,8 @@ function reifyIrExpression(expr: o.Expression): o.Expression {
return ng.pipeBindV(expr.targetSlot.slot!, expr.varOffset!, expr.args);
case ir.ExpressionKind.SanitizerExpr:
return o.importExpr(sanitizerIdentifierMap.get(expr.fn)!);
case ir.ExpressionKind.TrustedValueFnExpr:
return o.importExpr(trustedValueFnIdentifierMap.get(expr.fn)!);
case ir.ExpressionKind.SlotLiteralExpr:
return o.literal(expr.slot.slot!);
default:

View file

@ -15,25 +15,41 @@ import {createOpXrefMap} from '../util/elements';
/**
* Mapping of security contexts to sanitizer function for that context.
*/
const sanitizers = new Map<SecurityContext, ir.SanitizerFn|null>([
const sanitizers = new Map<SecurityContext, ir.SanitizerFn>([
[SecurityContext.HTML, ir.SanitizerFn.Html], [SecurityContext.SCRIPT, ir.SanitizerFn.Script],
[SecurityContext.STYLE, ir.SanitizerFn.Style], [SecurityContext.URL, ir.SanitizerFn.Url],
[SecurityContext.RESOURCE_URL, ir.SanitizerFn.ResourceUrl]
]);
/**
* Mapping of security contexts to trusted value function for that context.
*/
const trustedValueFns = new Map<SecurityContext, ir.TrustedValueFn>([
[SecurityContext.HTML, ir.TrustedValueFn.Html],
[SecurityContext.RESOURCE_URL, ir.TrustedValueFn.ResourceUrl]
]);
/**
* Resolves sanitization functions for ops that need them.
*/
export function resolveSanitizers(job: ComponentCompilationJob): void {
for (const unit of job.units) {
const elements = createOpXrefMap(unit);
let sanitizerFn: ir.SanitizerFn|null;
for (const op of unit.create) {
if (op.kind === ir.OpKind.ExtractedAttribute) {
const trustedValueFn = trustedValueFns.get(op.securityContext) ?? null;
op.trustedValueFn =
trustedValueFn !== null ? new ir.TrustedValueFnExpr(trustedValueFn) : null;
}
}
for (const op of unit.update) {
switch (op.kind) {
case ir.OpKind.Property:
case ir.OpKind.Attribute:
sanitizerFn = sanitizers.get(op.securityContext) || null;
op.sanitizer = sanitizerFn ? new ir.SanitizerExpr(sanitizerFn) : null;
const sanitizerFn = sanitizers.get(op.securityContext) ?? null;
op.sanitizer = sanitizerFn !== null ? new ir.SanitizerExpr(sanitizerFn) : null;
// If there was no sanitization function found based on the security context of an
// attribute/property, check whether this attribute/property is one of the
// security-sensitive <iframe> attributes (and that the current element is actually an