refactor(compiler): add support for sanitizing properties and attribu…

…tes (#51156) Sets sanitizer functions when attempting to set sensitive properties and attributes PR Close #51156
angular · Jul 28, 2023 · c1052cf · c1052cf
1 parent e4ae634
commit c1052cf
Show file tree

Hide file tree

Showing 12 changed files with 352 additions and 120 deletions.
diff --git a/...er-cli/test/compliance/test_cases/r3_view_compiler_styling/style_bindings/TEST_CASES.json b/...er-cli/test/compliance/test_cases/r3_view_compiler_styling/style_bindings/TEST_CASES.json
@@ -3,88 +3,63 @@
   "cases": [
     {
       "description": "should create style instructions on the element",
-      "inputFiles": [
-        "style_binding.ts"
-      ],
+      "inputFiles": ["style_binding.ts"],
       "expectations": [
         {
           "failureMessage": "Incorrect template",
-          "files": [
-            "style_binding.js"
-          ]
+          "files": ["style_binding.js"]
         }
       ]
     },
     {
       "description": "should correctly count the total slots required when style/class bindings include interpolation",
-      "inputFiles": [
-        "binding_slots.ts"
-      ],
+      "inputFiles": ["binding_slots.ts"],
       "expectations": [
         {
           "failureMessage": "Incorrect template",
-          "files": [
-            "binding_slots.js"
-          ]
+          "files": ["binding_slots.js"]
         }
       ]
     },
     {
       "description": "should place initial, multi, singular and application followed by attribute style instructions in the template code in that order",
-      "inputFiles": [
-        "binding_slots_interpolations.ts"
-      ],
+      "inputFiles": ["binding_slots_interpolations.ts"],
       "expectations": [
         {
           "failureMessage": "Incorrect template",
-          "files": [
-            "binding_slots_interpolations.js"
-          ]
+          "files": ["binding_slots_interpolations.js"]
         }
-      ],
-      "skipForTemplatePipeline": true
+      ]
     },
     {
       "description": "should assign a sanitizer instance to the element style allocation instruction if any url-based properties are detected",
-      "inputFiles": [
-        "style_ordering.ts"
-      ],
+      "inputFiles": ["style_ordering.ts"],
       "expectations": [
         {
           "failureMessage": "Incorrect template",
-          "files": [
-            "style_ordering.js"
-          ]
+          "files": ["style_ordering.js"]
         }
       ]
     },
     {
       "description": "should support [style.foo.suffix] style bindings with a suffix",
-      "inputFiles": [
-        "style_binding_suffixed.ts"
-      ],
+      "inputFiles": ["style_binding_suffixed.ts"],
       "expectations": [
         {
           "failureMessage": "Incorrect template",
-          "files": [
-            "style_binding_suffixed.js"
-          ]
+          "files": ["style_binding_suffixed.js"]
         }
       ]
     },
     {
       "description": "should not create instructions for empty style bindings",
-      "inputFiles": [
-        "empty_style_bindings.ts"
-      ],
+      "inputFiles": ["empty_style_bindings.ts"],
       "expectations": [
         {
           "failureMessage": "Incorrect template",
-          "files": [
-            "empty_style_bindings.js"
-          ]
+          "files": ["empty_style_bindings.js"]
         }
       ]
     }
   ]
-}
+}
diff --git a/packages/compiler/src/template/pipeline/ir/src/enums.ts b/packages/compiler/src/template/pipeline/ir/src/enums.ts
@@ -246,6 +246,11 @@ export enum ExpressionKind {
    * A reference to a temporary variable.
    */
   ReadTemporaryExpr,
+
+  /**
+   * An expression representing a sanitizer function.
+   */
+  SanitizerExpr,
 }
 
 /**
@@ -277,3 +282,15 @@ export enum CompatibilityMode {
   Normal,
   TemplateDefinitionBuilder,
 }
+
+/**
+ * Represents functions used to sanitize different pieces of a template.
+ */
+export enum SanitizerFn {
+  Html,
+  Script,
+  Style,
+  Url,
+  ResourceUrl,
+  IframeAttribute,
+}
diff --git a/packages/compiler/src/template/pipeline/ir/src/expression.ts b/packages/compiler/src/template/pipeline/ir/src/expression.ts
@@ -9,7 +9,7 @@
 import * as o from '../../../../output/output_ast';
 import type {ParseSourceSpan} from '../../../../parse_util';
 
-import {ExpressionKind, OpKind} from './enums';
+import {ExpressionKind, OpKind, SanitizerFn} from './enums';
 import {ConsumesVarsTrait, UsesSlotIndex, UsesSlotIndexTrait, UsesVarOffset, UsesVarOffsetTrait} from './traits';
 
 import type {XrefId} from './operations';
@@ -19,10 +19,11 @@ import {Interpolation, type UpdateOp} from './ops/update';
 /**
  * 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;
+export type Expression =
+    LexicalReadExpr|ReferenceExpr|ContextExpr|NextContextExpr|GetCurrentViewExpr|RestoreViewExpr|
+    ResetViewExpr|ReadVariableExpr|PureFunctionExpr|PureFunctionParameterExpr|PipeBindingExpr|
+    PipeBindingVariadicExpr|SafePropertyReadExpr|SafeKeyedReadExpr|SafeInvokeFunctionExpr|EmptyExpr|
+    AssignTemporaryExpr|ReadTemporaryExpr|SanitizerExpr;
 
 /**
  * Transformer type which converts expressions into general `o.Expression`s (which may be an
@@ -709,6 +710,30 @@ export class ReadTemporaryExpr extends ExpressionBase {
   }
 }
 
+export class SanitizerExpr extends ExpressionBase {
+  override readonly kind = ExpressionKind.SanitizerExpr;
+
+  constructor(public fn: SanitizerFn) {
+    super();
+  }
+
+  override visitExpression(visitor: o.ExpressionVisitor, context: any): any {}
+
+  override isEquivalent(e: Expression): boolean {
+    return e instanceof SanitizerExpr && e.fn === this.fn;
+  }
+
+  override isConstant() {
+    return true;
+  }
+
+  override clone(): SanitizerExpr {
+    return new SanitizerExpr(this.fn);
+  }
+
+  override transformInternalExpressions(): void {}
+}
+
 /**
  * Visits all `Expression`s in the AST of `op` with the `visitor` function.
  */
@@ -742,12 +767,10 @@ function transformExpressionsInInterpolation(
 export function transformExpressionsInOp(
     op: CreateOp|UpdateOp, transform: ExpressionTransform, flags: VisitorContextFlag): void {
   switch (op.kind) {
-    case OpKind.Property:
     case OpKind.StyleProp:
     case OpKind.StyleMap:
     case OpKind.ClassProp:
     case OpKind.ClassMap:
-    case OpKind.Attribute:
     case OpKind.Binding:
     case OpKind.HostProperty:
       if (op.expression instanceof Interpolation) {
@@ -756,6 +779,16 @@ export function transformExpressionsInOp(
         op.expression = transformExpressionsInExpression(op.expression, transform, flags);
       }
       break;
+    case OpKind.Property:
+    case OpKind.Attribute:
+      if (op.expression instanceof Interpolation) {
+        transformExpressionsInInterpolation(op.expression, transform, flags);
+      } else {
+        op.expression = transformExpressionsInExpression(op.expression, transform, flags);
+      }
+      op.sanitizer =
+          op.sanitizer && transformExpressionsInExpression(op.sanitizer, transform, flags);
+      break;
     case OpKind.InterpolateText:
       transformExpressionsInInterpolation(op.interpolation, transform, flags);
       break;

diff --git a/packages/compiler/src/template/pipeline/ir/src/ops/update.ts b/packages/compiler/src/template/pipeline/ir/src/ops/update.ts
@@ -6,6 +6,7 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
+import {SecurityContext} from '../../../../../core';
 import * as o from '../../../../../output/output_ast';
 import {ParseSourceSpan} from '../../../../../parse_util';
 import {BindingKind} from '../element';
@@ -71,32 +72,59 @@ export class Interpolation {
 export interface BindingOp extends Op<UpdateOp> {
   kind: OpKind.Binding;
 
+  /**
+   * Reference to the element on which the property is bound.
+   */
   target: XrefId;
 
+  /**
+   *  The kind of binding represented by this op.
+   */
   bindingKind: BindingKind;
+
+  /**
+   *  The name of this binding.
+   */
   name: string;
+
+  /**
+   * Expression which is bound to the property.
+   */
   expression: o.Expression|Interpolation;
-  sourceSpan: ParseSourceSpan;
-  isTemplate: boolean;
+
   /**
    * The unit of the bound value.
    */
   unit: string|null;
+
+  /**
+   * The security context of the binding.
+   */
+  securityContext: SecurityContext;
+
+  /**
+   * Whether this binding is on a template.
+   */
+  isTemplate: boolean;
+
+  sourceSpan: ParseSourceSpan;
 }
 
 /**
  * Create a `BindingOp`, not yet transformed into a particular type of binding.
  */
 export function createBindingOp(
     target: XrefId, kind: BindingKind, name: string, expression: o.Expression|Interpolation,
-    unit: string|null, isTemplate: boolean, sourceSpan: ParseSourceSpan): BindingOp {
+    unit: string|null, securityContext: SecurityContext, isTemplate: boolean,
+    sourceSpan: ParseSourceSpan): BindingOp {
   return {
     kind: OpKind.Binding,
     bindingKind: kind,
     target,
     name,
     expression,
     unit,
+    securityContext,
     isTemplate,
     sourceSpan,
     ...NEW_OP,
@@ -129,6 +157,19 @@ export interface PropertyOp extends Op<UpdateOp>, ConsumesVarsTrait, DependsOnSl
    */
   isAnimationTrigger: boolean;
 
+  /**
+   * The security context of the binding.
+   */
+  securityContext: SecurityContext;
+
+  /**
+   * The sanitizer for this property.
+   */
+  sanitizer: o.Expression|null;
+
+  /**
+   * Whether this binding is on a template.
+   */
   isTemplate: boolean;
 
   sourceSpan: ParseSourceSpan;
@@ -139,7 +180,7 @@ export interface PropertyOp extends Op<UpdateOp>, ConsumesVarsTrait, DependsOnSl
  */
 export function createPropertyOp(
     target: XrefId, name: string, expression: o.Expression|Interpolation,
-    isAnimationTrigger: boolean, isTemplate: boolean,
+    isAnimationTrigger: boolean, securityContext: SecurityContext, isTemplate: boolean,
 
     sourceSpan: ParseSourceSpan): PropertyOp {
   return {
@@ -148,6 +189,8 @@ export function createPropertyOp(
     name,
     expression,
     isAnimationTrigger,
+    securityContext,
+    sanitizer: null,
     isTemplate,
     sourceSpan,
     ...TRAIT_DEPENDS_ON_SLOT_CONTEXT,
@@ -333,6 +376,19 @@ export interface AttributeOp extends Op<UpdateOp> {
    */
   expression: o.Expression|Interpolation;
 
+  /**
+   * The security context of the binding.
+   */
+  securityContext: SecurityContext;
+
+  /**
+   * The sanitizer for this attribute.
+   */
+  sanitizer: o.Expression|null;
+
+  /**
+   * Whether this binding is on a template.
+   */
   isTemplate: boolean;
 
   sourceSpan: ParseSourceSpan;
@@ -342,13 +398,16 @@ export interface AttributeOp extends Op<UpdateOp> {
  * Create an `AttributeOp`.
  */
 export function createAttributeOp(
-    target: XrefId, name: string, expression: o.Expression|Interpolation, isTemplate: boolean,
+    target: XrefId, name: string, expression: o.Expression|Interpolation,
+    securityContext: SecurityContext, isTemplate: boolean,
     sourceSpan: ParseSourceSpan): AttributeOp {
   return {
     kind: OpKind.Attribute,
     target,
     name,
     expression,
+    securityContext,
+    sanitizer: null,
     isTemplate,
     sourceSpan,
     ...TRAIT_DEPENDS_ON_SLOT_CONTEXT,