fix(core): hardening rules related to the attribute order on iframe e…

…lements (#47935)

This commit updates the logic related to the attribute order on iframes and makes the rules more strict. There is a set of iframe attributes that may affect the behavior of an iframe, this change enforces that these attributes are applied before an `src` or `srcdoc` attributes are applied to an iframe, so that they are taken into account.

If Angular detects that some of the attributes are set after the `src` or `srcdoc`, it throws an error message, which contains the name of ann attribute that is causing the problem and the name of a Component where an iframe is located. In most cases, it should be enough to change the order of attributes in a template to move the `src` or `srcdoc` ones to the very end.

BREAKING CHANGE:

Existing iframe usages may have `src` or `srcdoc` preceding other attributes. Such usages may need to be updated to ensure compliance with the new stricter rules around iframe bindings.

PR Close #47935

goldens/public-api/core/errors.md

@@ -101,6 +101,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,

goldens/size-tracking/aio-payloads.json

@@ -12,7 +12,7 @@
   "aio-local": {
     "uncompressed": {
       "runtime": 4325,
-      "main": 455228,
+      "main": 456041,
       "polyfills": 33952,
       "styles": 73964,
       "light-theme": 78157,

goldens/size-tracking/integration-payloads.json

@@ -48,7 +48,7 @@
   "animations": {
     "uncompressed": {
       "runtime": 1070,
-      "main": 155920,
+      "main": 156446,
       "polyfills": 33814
     }
   },

packages/compiler-cli/test/compliance/test_cases/r3_compiler_compliance/elements/security_sensitive_constant_attributes.js

@@ -6,7 +6,7 @@ consts: [
 ],
 template: function MyComponent_Template(rf, ctx) {
   if (rf & 1) {
-    $r3$.ɵɵelement(0, "embed", 0)(1, "iframe", 1)(2, "object", 2)(3, "embed", 0)(4, "img", 3);
+    $r3$.ɵɵelement(0, "embed", 0)(1, "iframe", 1, null, i0.ɵɵvalidateIframeStaticAttributes)(2, "object", 2)(3, "embed", 0)(4, "img", 3);
   }
   …
 }

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

@@ -7506,6 +7506,218 @@ 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} from '@angular/core';
+
+                @Component({
+                  template: \`
+                    <iframe src="http://angular.io"
+                      [sandbox]="''" [attr.allow]="''"
+                      [title]="'Hi!'"
+                    ></iframe>
+                  \`
+                })
+                export class SomeComponent {}
+              `);
+
+           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)');
+
+           // Expect an extra validation function on the `element` instruction for an <iframe>
+           // to validate static attributes.
+           expect(jsContents)
+               .toContain('ɵɵelement(0, "iframe", 0, null, i0.ɵɵvalidateIframeStaticAttributes)');
+         });
+
+      it('should generate attribute and property bindings with a validator fn when on <iframe> ' +
+             'with an *ngIf on it as well',
+         () => {
+           env.write('test.ts', `
+                import {Component} from '@angular/core';
+
+                @Component({
+                  template: \`
+                    <iframe
+                      *ngIf="visible"
+                      src="http://angular.io"
+                      [sandbox]="''"
+                    ></iframe>
+                  \`
+                })
+                export class SomeComponent {
+                  visible = true;
+                }
+              `);
+
+           env.driveMain();
+           const jsContents = env.getContents('test.js');
+
+           // Expect an extra validation function on the `element` instruction for an <iframe>
+           // to validate static attributes.
+           expect(jsContents)
+               .toContain('ɵɵelement(0, "iframe", 1, null, i0.ɵɵvalidateIframeStaticAttributes)');
+
+           // The `template` instruction that represents an <iframe> remains as is
+           // (without additional validation fns).
+           expect(jsContents)
+               .toContain('ɵɵtemplate(0, SomeComponent_iframe_0_Template, 1, 1, "iframe", 0)');
+         });
+
+      it('should generate an element instruction with a validator function for <iframe>s ' +
+             '(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)');
+
+           // Expect an extra validation function on the `element` instruction for an <iframe>
+           // to validate static attributes.
+           expect(jsContents)
+               .toContain('ɵɵelement(0, "IFRAME", 0, null, i0.ɵɵvalidateIframeStaticAttributes)');
+         });
+
+      it('should include static attribute validation fn when on <iframe> ' +
+             '(even if there are no static attributes)',
+         () => {
+           env.write('test.ts', `
+                import {Component} from '@angular/core';
+
+                @Component({
+                  template: \`
+                    <iframe [src]="'http://angular.io'"></iframe>
+                  \`
+                })
+                export class SomeComponent {}
+              `);
+
+           env.driveMain();
+           const jsContents = env.getContents('test.js');
+
+           // Expect an extra validation function on the `element` instruction for an <iframe>
+           // to validate static attributes even if there are no static attributes present
+           // on an <iframe>. There might be directives with static host attributes mathcing
+           // this <iframe>, the validation function will be used to check the final set of
+           // static attributes (from all directives and an element itself).
+           expect(jsContents)
+               .toContain('ɵɵelement(0, "iframe", 0, null, i0.ɵɵvalidateIframeStaticAttributes)');
+         });
+
+      it('should *not* generate a validator fn for attribute and property bindings when *not* on <iframe>',
+         () => {
+           env.write('test.ts', `
+                import {Component, Directive} from '@angular/core';
+
+                @Directive({
+                  standalone: true,
+                  selector: '[sandbox]',
+                  inputs: ['sandbox']
+                })
+                class Dir {}
+
+                @Component({
+                  standalone: true,
+                  imports: [Dir],
+                  template: \`
+                    <div [sandbox]="''" [title]="'Hi!'"></div>
+                  \`
+                })
+                export class SomeComponent {}
+              `);
+
+           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!")');
+
+           // No extra validation fn on an `element` instruction too, since it's
+           // not an <iframe>.
+           expect(jsContents).toContain('ɵɵelement(0, "div", 0)');
+         });
+
+      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

@@ -346,4 +346,8 @@ 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};
+  static validateIframeStaticAttributes:
+      o.ExternalReference = {name: 'ɵɵvalidateIframeStaticAttributes', 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';
@@ -574,6 +575,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';
@@ -675,6 +676,16 @@ export class TemplateDefinitionBuilder implements t.Visitor<void>, LocalResolver
     const refs = this.prepareRefsArray(element.references);
     parameters.push(this.addToConsts(refs));
 
+    // If this element is an <iframe>, append an extra validation
+    // function, which would be invoked at runtime to make sure that
+    // all security-sensitive attributes defined statically (both on
+    // the element itself as well as on all matched directives) are
+    // set on the underlying <iframe> *before* setting its `src` or
+    // `srcdoc` (otherwise they'd not be taken into account).
+    if (isIframeElement(element.name)) {
+      parameters.push(o.importExpr(R3.validateIframeStaticAttributes));
+    }
+
     const wasInNamespace = this._namespace;
     const currentNamespace = this.getNamespaceInstruction(namespaceKey);
 
@@ -783,8 +794,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);
 
@@ -2211,6 +2233,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

@@ -76,3 +76,28 @@ 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 before setting the `src` or `srcdoc` attribute value.
+ * This ensures that all security-sensitive attributes are taken into account
+ * while creating an instance of an `<iframe>` at runtime.
+ *
+ * Keep this list in sync with the `IFRAME_SECURITY_SENSITIVE_ATTRS` token
+ * from the `packages/core/src/sanitization/iframe_attrs_validation.ts` script.
+ *
+ * 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', 'loading', '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());
+}