[13.3.x] fix(core): hardening attribute and property binding rules for <iframe> elements

aio/content/errors/NG0910.md

@@ -0,0 +1,71 @@
+@name Unsafe bindings on an iframe element
+@category runtime
+@shortDescription Unsafe bindings on an iframe element
+
+@description
+You see this error when Angular detects an attribute binding or a property binding on an `<iframe>` element using the following property names:
+
+* sandbox
+* allow
+* allowFullscreen
+* referrerPolicy
+* csp
+* fetchPriority
+
+The mentioned attributes affect the security model setup for `<iframe>`s
+and it's important to apply them before setting the `src` or `srcdoc` attributes.
+To enforce that, Angular requires these attributes to be set on `<iframe>`s as
+static attributes, so the values are set at the element creation time and they
+remain the same throughout the lifetime of an `<iframe>` instance.
+
+The error is thrown when a property binding with one of the mentioned attribute names is used:
+```html
+<iframe [sandbox]="'allow-scripts'" src="..."></iframe>
+```
+
+or when it's an attribute bindings:
+
+```html
+<iframe [attr.sandbox]="'allow-scripts'" src="..."></iframe>
+```
+
+Also, the error is thrown when a similar pattern is used in Directive's host bindings:
+
+```typescript
+@Directive({
+  selector: 'iframe',
+  host: {
+    '[sandbox]': `'allow-scripts'`,
+    '[attr.sandbox]': `'allow-scripts'`,
+  }
+})
+class IframeDirective {}
+```
+
+@debugging
+
+The error message includes the name of the component with the template where
+an `<iframe>` element with unsafe bindings is located.
+
+The recommended solution is to use the mentioned attributes as static ones, for example:
+
+```html
+<iframe sandbox="allow-scripts" src="..."></iframe>
+```
+
+If you need to have different values for these attributes (depending on various conditions),
+you can use an `*ngIf` or an `*ngSwitch` on an `<iframe>` element:
+
+```html
+<iframe *ngIf="someConditionA" sandbox="allow-scripts" src="..."></iframe>
+<iframe *ngIf="someConditionB" sandbox="allow-forms" src="..."></iframe>
+<iframe *ngIf="someConditionC" sandbox="allow-popups" src="..."></iframe>
+```
+
+<!-- links -->
+
+<!-- external links -->
+
+<!-- end links -->
+
+@reviewed 2022-05-27

goldens/public-api/core/errors.md

@@ -67,6 +67,8 @@ export const enum RuntimeErrorCode {
     // (undocumented)
     UNKNOWN_ELEMENT = 304,
     // (undocumented)
+    UNSAFE_IFRAME_ATTRS = -910,
+    // (undocumented)
     UNSAFE_VALUE_IN_RESOURCE_URL = 904,
     // (undocumented)
     UNSAFE_VALUE_IN_SCRIPT = 905,

packages/compiler-cli/test/ngtsc/ngtsc_spec.ts

@@ -6990,6 +6990,158 @@ function allTests(os: string) {
       });
     });
 
+    describe('iframe processing', () => {
+      it('should generate attribute and property bindings with a validator fn when on <iframe>',
+         () => {
+           env.write('test.ts', `
+                import {Component, Directive, NgModule} from '@angular/core';
+
+                @Directive({
+                  selector: 'iframe',
+                  inputs: ['sandbox']
+                })
+                class SomeDirective {}
+
+                @Component({
+                  template: \`
+                    <iframe src="http://angular.io"
+                      [sandbox]="''" [attr.allow]="''"
+                      [title]="'Hi!'"
+                    ></iframe>
+                  \`
+                })
+                class SomeComponent {}
+
+                @NgModule({
+                  declarations: [SomeDirective, SomeComponent]
+                })
+                class SomeNgModule {}
+              `);
+
+           env.driveMain();
+           const jsContents = env.getContents('test.js');
+
+           // Only `sandbox` has an extra validation fn (since it's security-sensitive),
+           // the `title` property doesn't have an extra validation fn.
+           expect(jsContents)
+               .toContain(
+                   'ɵɵproperty("sandbox", "", i0.ɵɵvalidateIframeAttribute)("title", "Hi!")');
+
+           // The `allow` property is also security-sensitive, thus an extra validation fn.
+           expect(jsContents).toContain('ɵɵattribute("allow", "", i0.ɵɵvalidateIframeAttribute)');
+         });
+
+      it('should generate an attribute binding instruction with a validator function ' +
+             '(making sure it\'s case-insensitive, since this is allowed in Angular templates)',
+         () => {
+           env.write('test.ts', `
+              import {Component} from '@angular/core';
+
+              @Component({
+                template: \`
+                  <IFRAME
+                    src="http://angular.io"
+                    [attr.SANDBOX]="''"
+                  ></IFRAME>
+                \`
+              })
+              export class SomeComponent {}
+            `);
+
+           env.driveMain();
+           const jsContents = env.getContents('test.js');
+
+           // Make sure that the `sandbox` has an extra validation fn,
+           // and the check is case-insensitive (since the `setAttribute` DOM API
+           // is case-insensitive as well).
+           expect(jsContents).toContain('ɵɵattribute("SANDBOX", "", i0.ɵɵvalidateIframeAttribute)');
+         });
+
+      it('should *not* generate a validator fn for attribute and property bindings when *not* on <iframe>',
+         () => {
+           env.write('test.ts', `
+                import {Component, Directive, NgModule} from '@angular/core';
+
+                @Directive({
+                  selector: '[sandbox]',
+                  inputs: ['sandbox']
+                })
+                class Dir {}
+
+                @Component({
+                  imports: [Dir],
+                  template: \`
+                    <div [sandbox]="''" [title]="'Hi!'"></div>
+                  \`
+                })
+                export class SomeComponent {}
+
+                @NgModule({
+                  declarations: [Dir, SomeComponent]
+                })
+                class SomeNgModule {}
+              `);
+
+           env.driveMain();
+           const jsContents = env.getContents('test.js');
+
+           // Note: no extra validation fn, since a security-sensitive attribute is *not* on an
+           // <iframe>.
+           expect(jsContents).toContain('ɵɵproperty("sandbox", "")("title", "Hi!")');
+         });
+
+      it('should generate a validator fn for attribute and property host bindings on a directive',
+         () => {
+           env.write('test.ts', `
+              import {Directive} from '@angular/core';
+
+              @Directive({
+                host: {
+                  '[sandbox]': "''",
+                  '[attr.allow]': "''",
+                  'src': 'http://angular.io'
+                }
+              })
+              export class SomeDir {}
+            `);
+
+           env.driveMain();
+           const jsContents = env.getContents('test.js');
+
+           // The `sandbox` is potentially a security-sensitive attribute of an <iframe>.
+           // Generate an extra validation function to invoke at runtime, which would
+           // check if an underlying host element is an <iframe>.
+           expect(jsContents)
+               .toContain('ɵɵhostProperty("sandbox", "", i0.ɵɵvalidateIframeAttribute)');
+
+           // Similar to the above, but for an attribute binding (host attributes are
+           // represented via `ɵɵattribute`).
+           expect(jsContents).toContain('ɵɵattribute("allow", "", i0.ɵɵvalidateIframeAttribute)');
+         });
+
+      it('should generate a validator fn for attribute host bindings on a directive ' +
+             '(making sure the check is case-insensitive)',
+         () => {
+           env.write('test.ts', `
+              import {Directive} from '@angular/core';
+
+              @Directive({
+                host: {
+                  '[attr.SANDBOX]': "''"
+                }
+              })
+              export class SomeDir {}
+            `);
+
+           env.driveMain();
+           const jsContents = env.getContents('test.js');
+
+           // Make sure that we generate a validation fn for the `sandbox` attribute,
+           // even when it was declared as `SANDBOX`.
+           expect(jsContents).toContain('ɵɵattribute("SANDBOX", "", i0.ɵɵvalidateIframeAttribute)');
+         });
+    });
+
     describe('undecorated providers', () => {
       it('should error when an undecorated class, with a non-trivial constructor, is provided directly in a module',
          () => {

packages/compiler/src/render3/r3_identifiers.ts

@@ -336,4 +336,6 @@ export class Identifiers {
   static trustConstantHtml: o.ExternalReference = {name: 'ɵɵtrustConstantHtml', moduleName: CORE};
   static trustConstantResourceUrl:
       o.ExternalReference = {name: 'ɵɵtrustConstantResourceUrl', moduleName: CORE};
+  static validateIframeAttribute:
+      o.ExternalReference = {name: 'ɵɵvalidateIframeAttribute', moduleName: CORE};
 }

packages/compiler/src/render3/view/compiler.ts

@@ -12,6 +12,7 @@ import * as core from '../../core';
 import {AST, ParsedEvent, ParsedEventType, ParsedProperty} from '../../expression_parser/ast';
 import * as o from '../../output/output_ast';
 import {ParseError, ParseSourceSpan, sanitizeIdentifier} from '../../parse_util';
+import {isIframeSecuritySensitiveAttr} from '../../schema/dom_security_schema';
 import {CssSelector} from '../../selector';
 import {ShadowCss} from '../../shadow_css';
 import {BindingParser} from '../../template_parser/binding_parser';
@@ -551,6 +552,19 @@ function createHostBindingsFunction(
     const instructionParams = [o.literal(bindingName), bindingExpr.currValExpr];
     if (sanitizerFn) {
       instructionParams.push(sanitizerFn);
+    } else {
+      // If there was no sanitization function found based on the security context
+      // of an attribute/property binding - check whether this attribute/property is
+      // one of the security-sensitive <iframe> attributes.
+      // Note: for host bindings defined on a directive, we do not try to find all
+      // possible places where it can be matched, so we can not determine whether
+      // the host element is an <iframe>. In this case, if an attribute/binding
+      // name is in the `IFRAME_SECURITY_SENSITIVE_ATTRS` set - append a validation
+      // function, which would be invoked at runtime and would have access to the
+      // underlying DOM element, check if it's an <iframe> and if so - runs extra checks.
+      if (isIframeSecuritySensitiveAttr(bindingName)) {
+        instructionParams.push(o.importExpr(R3.validateIframeAttribute));
+      }
     }
 
     updateVariables.push(...bindingExpr.stmts);

packages/compiler/src/render3/view/template.ts

@@ -23,6 +23,7 @@ import {mapLiteral} from '../../output/map_util';
 import * as o from '../../output/output_ast';
 import {ParseError, ParseSourceSpan, sanitizeIdentifier} from '../../parse_util';
 import {DomElementSchemaRegistry} from '../../schema/dom_element_schema_registry';
+import {isIframeSecuritySensitiveAttr} from '../../schema/dom_security_schema';
 import {isTrustedTypesSink} from '../../schema/trusted_types_sinks';
 import {CssSelector} from '../../selector';
 import {BindingParser} from '../../template_parser/binding_parser';
@@ -765,8 +766,19 @@ export class TemplateDefinitionBuilder implements t.Visitor<void>, LocalResolver
           const params: any[] = [];
           const [attrNamespace, attrName] = splitNsName(input.name);
           const isAttributeBinding = inputType === BindingType.Attribute;
-          const sanitizationRef = resolveSanitizationFn(input.securityContext, isAttributeBinding);
-          if (sanitizationRef) params.push(sanitizationRef);
+          let sanitizationRef = resolveSanitizationFn(input.securityContext, isAttributeBinding);
+          if (!sanitizationRef) {
+            // 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 <iframe>).
+            if (isIframeElement(element.name) && isIframeSecuritySensitiveAttr(input.name)) {
+              sanitizationRef = o.importExpr(R3.validateIframeAttribute);
+            }
+          }
+          if (sanitizationRef) {
+            params.push(sanitizationRef);
+          }
           if (attrNamespace) {
             const namespaceLiteral = o.literal(attrNamespace);
 
@@ -2195,6 +2207,10 @@ function isTextNode(node: t.Node): boolean {
   return node instanceof t.Text || node instanceof t.BoundText || node instanceof t.Icu;
 }
 
+function isIframeElement(tagName: string): boolean {
+  return tagName.toLowerCase() === 'iframe';
+}
+
 function hasTextChildrenOnly(children: t.Node[]): boolean {
   return children.every(isTextNode);
 }

packages/compiler/src/schema/dom_security_schema.ts

@@ -62,3 +62,25 @@ export function SECURITY_SCHEMA(): {[k: string]: SecurityContext} {
 function registerContext(ctx: SecurityContext, specs: string[]) {
   for (const spec of specs) _SECURITY_SCHEMA[spec.toLowerCase()] = ctx;
 }
+
+/**
+ * The set of security-sensitive attributes of an `<iframe>` that *must* be
+ * applied as a static attribute only. This ensures that all security-sensitive
+ * attributes are taken into account while creating an instance of an `<iframe>`
+ * at runtime.
+ *
+ * Note: avoid using this set directly, use the `isIframeSecuritySensitiveAttr` function
+ * in the code instead.
+ */
+export const IFRAME_SECURITY_SENSITIVE_ATTRS =
+    new Set(['sandbox', 'allow', 'allowfullscreen', 'referrerpolicy', 'csp', 'fetchpriority']);
+
+/**
+ * Checks whether a given attribute name might represent a security-sensitive
+ * attribute of an <iframe>.
+ */
+export function isIframeSecuritySensitiveAttr(attrName: string): boolean {
+  // The `setAttribute` DOM API is case-insensitive, so we lowercase the value
+  // before checking it against a known security-sensitive attributes.
+  return IFRAME_SECURITY_SENSITIVE_ATTRS.has(attrName.toLowerCase());
+}

packages/compiler/test/security_spec.ts

@@ -0,0 +1,30 @@
+/**
+ * @license
+ * Copyright Google LLC 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 {IFRAME_SECURITY_SENSITIVE_ATTRS, SECURITY_SCHEMA} from '../src/schema/dom_security_schema';
+
+
+describe('security-related tests', () => {
+  it('should have no overlap between `IFRAME_SECURITY_SENSITIVE_ATTRS` and `SECURITY_SCHEMA`',
+     () => {
+       // The `IFRAME_SECURITY_SENSITIVE_ATTRS` and `SECURITY_SCHEMA` tokens configure sanitization
+       // and validation rules and used to pick the right sanitizer function.
+       // This test verifies that there is no overlap between two sets of rules to flag
+       // a situation when 2 sanitizer functions may be needed at the same time (in which
+       // case, compiler logic should be extended to support that).
+       const schema = new Set();
+       Object.keys(SECURITY_SCHEMA()).forEach((key: string) => schema.add(key.toLowerCase()));
+       let hasOverlap = false;
+       IFRAME_SECURITY_SENSITIVE_ATTRS.forEach(attr => {
+         if (schema.has('*|' + attr) || schema.has('iframe|' + attr)) {
+           hasOverlap = true;
+         }
+       });
+       expect(hasOverlap).toBeFalse();
+     });
+});

packages/core/src/core_render3_private_export.ts

@@ -269,6 +269,9 @@ export {
   ɵɵtrustConstantHtml,
   ɵɵtrustConstantResourceUrl,
 } from './sanitization/sanitization';
+export {
+  ɵɵvalidateIframeAttribute,
+} from './sanitization/iframe_attrs_validation';
 export {
   noSideEffects as ɵnoSideEffects,
 } from './util/closure';

packages/core/src/errors.ts

@@ -62,7 +62,8 @@ export const enum RuntimeErrorCode {
   VIEW_ALREADY_ATTACHED = 902,
   INVALID_INHERITANCE = 903,
   UNSAFE_VALUE_IN_RESOURCE_URL = 904,
-  UNSAFE_VALUE_IN_SCRIPT = 905
+  UNSAFE_VALUE_IN_SCRIPT = 905,
+  UNSAFE_IFRAME_ATTRS = -910,
 }
 
 /**