Skip to content

Commit b871db5

Browse files
AndrewKushnirdylhunn
AndrewKushnir
authored and
dylhunn
committed
fix(core): hardening attribute and property binding rules for <iframe> elements
This commit updates the logic related to the attribute and property binding rules for <iframe> elements. There is a set of <iframe> attributes that may affect the behavior of an iframe and this change enforces that these attributes are only applied as static attributes, making sure that they are taken into account while creating an <iframe>. If Angular detects that some of the security-sensitive attributes are applied as an attribute or property binding, it throws an error message, which contains the name of an attribute that is causing the problem and the name of a Component where an iframe is located. BREAKING CHANGE: Existing iframe usages may have security-sensitive attributes applied as an attribute or property binding in a template or via host bindings in a directive. Such usages would require an update to ensure compliance with the new stricter rules around iframe bindings.
1 parent 64b8a8d commit b871db5

File tree

12 files changed

+914
-6
lines changed

12 files changed

+914
-6
lines changed

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

Lines changed: 152 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7548,6 +7548,158 @@ export const Foo = Foo__PRE_R3__;
75487548
});
75497549
});
75507550

7551+
describe('iframe processing', () => {
7552+
it('should generate attribute and property bindings with a validator fn when on <iframe>',
7553+
() => {
7554+
env.write('test.ts', `
7555+
import {Component, Directive, NgModule} from '@angular/core';
7556+
7557+
@Directive({
7558+
selector: 'iframe',
7559+
inputs: ['sandbox']
7560+
})
7561+
class SomeDirective {}
7562+
7563+
@Component({
7564+
template: \`
7565+
<iframe src="http://angular.io"
7566+
[sandbox]="''" [attr.allow]="''"
7567+
[title]="'Hi!'"
7568+
></iframe>
7569+
\`
7570+
})
7571+
class SomeComponent {}
7572+
7573+
@NgModule({
7574+
declarations: [SomeDirective, SomeComponent]
7575+
})
7576+
class SomeNgModule {}
7577+
`);
7578+
7579+
env.driveMain();
7580+
const jsContents = env.getContents('test.js');
7581+
7582+
// Only `sandbox` has an extra validation fn (since it's security-sensitive),
7583+
// the `title` property doesn't have an extra validation fn.
7584+
expect(jsContents)
7585+
.toContain(
7586+
'ɵɵproperty("sandbox", "", i0.ɵɵvalidateIframeAttribute)("title", "Hi!")');
7587+
7588+
// The `allow` property is also security-sensitive, thus an extra validation fn.
7589+
expect(jsContents).toContain('ɵɵattribute("allow", "", i0.ɵɵvalidateIframeAttribute)');
7590+
});
7591+
7592+
it('should generate an attribute binding instruction with a validator function ' +
7593+
'(making sure it\'s case-insensitive, since this is allowed in Angular templates)',
7594+
() => {
7595+
env.write('test.ts', `
7596+
import {Component} from '@angular/core';
7597+
7598+
@Component({
7599+
template: \`
7600+
<IFRAME
7601+
src="http://angular.io"
7602+
[attr.SANDBOX]="''"
7603+
></IFRAME>
7604+
\`
7605+
})
7606+
export class SomeComponent {}
7607+
`);
7608+
7609+
env.driveMain();
7610+
const jsContents = env.getContents('test.js');
7611+
7612+
// Make sure that the `sandbox` has an extra validation fn,
7613+
// and the check is case-insensitive (since the `setAttribute` DOM API
7614+
// is case-insensitive as well).
7615+
expect(jsContents).toContain('ɵɵattribute("SANDBOX", "", i0.ɵɵvalidateIframeAttribute)');
7616+
});
7617+
7618+
it('should *not* generate a validator fn for attribute and property bindings when *not* on <iframe>',
7619+
() => {
7620+
env.write('test.ts', `
7621+
import {Component, Directive, NgModule} from '@angular/core';
7622+
7623+
@Directive({
7624+
selector: '[sandbox]',
7625+
inputs: ['sandbox']
7626+
})
7627+
class Dir {}
7628+
7629+
@Component({
7630+
imports: [Dir],
7631+
template: \`
7632+
<div [sandbox]="''" [title]="'Hi!'"></div>
7633+
\`
7634+
})
7635+
export class SomeComponent {}
7636+
7637+
@NgModule({
7638+
declarations: [Dir, SomeComponent]
7639+
})
7640+
class SomeNgModule {}
7641+
`);
7642+
7643+
env.driveMain();
7644+
const jsContents = env.getContents('test.js');
7645+
7646+
// Note: no extra validation fn, since a security-sensitive attribute is *not* on an
7647+
// <iframe>.
7648+
expect(jsContents).toContain('ɵɵproperty("sandbox", "")("title", "Hi!")');
7649+
});
7650+
7651+
it('should generate a validator fn for attribute and property host bindings on a directive',
7652+
() => {
7653+
env.write('test.ts', `
7654+
import {Directive} from '@angular/core';
7655+
7656+
@Directive({
7657+
host: {
7658+
'[sandbox]': "''",
7659+
'[attr.allow]': "''",
7660+
'src': 'http://angular.io'
7661+
}
7662+
})
7663+
export class SomeDir {}
7664+
`);
7665+
7666+
env.driveMain();
7667+
const jsContents = env.getContents('test.js');
7668+
7669+
// The `sandbox` is potentially a security-sensitive attribute of an <iframe>.
7670+
// Generate an extra validation function to invoke at runtime, which would
7671+
// check if an underlying host element is an <iframe>.
7672+
expect(jsContents)
7673+
.toContain('ɵɵhostProperty("sandbox", "", i0.ɵɵvalidateIframeAttribute)');
7674+
7675+
// Similar to the above, but for an attribute binding (host attributes are
7676+
// represented via `ɵɵattribute`).
7677+
expect(jsContents).toContain('ɵɵattribute("allow", "", i0.ɵɵvalidateIframeAttribute)');
7678+
});
7679+
7680+
it('should generate a validator fn for attribute host bindings on a directive ' +
7681+
'(making sure the check is case-insensitive)',
7682+
() => {
7683+
env.write('test.ts', `
7684+
import {Directive} from '@angular/core';
7685+
7686+
@Directive({
7687+
host: {
7688+
'[attr.SANDBOX]': "''"
7689+
}
7690+
})
7691+
export class SomeDir {}
7692+
`);
7693+
7694+
env.driveMain();
7695+
const jsContents = env.getContents('test.js');
7696+
7697+
// Make sure that we generate a validation fn for the `sandbox` attribute,
7698+
// even when it was declared as `SANDBOX`.
7699+
expect(jsContents).toContain('ɵɵattribute("SANDBOX", "", i0.ɵɵvalidateIframeAttribute)');
7700+
});
7701+
});
7702+
75517703
describe('undecorated providers', () => {
75527704
it('should error when an undecorated class, with a non-trivial constructor, is provided directly in a module',
75537705
() => {

‎packages/compiler/src/render3/r3_identifiers.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -336,4 +336,6 @@ export class Identifiers {
336336
static trustConstantHtml: o.ExternalReference = {name: 'ɵɵtrustConstantHtml', moduleName: CORE};
337337
static trustConstantResourceUrl:
338338
o.ExternalReference = {name: 'ɵɵtrustConstantResourceUrl', moduleName: CORE};
339+
static validateIframeAttribute:
340+
o.ExternalReference = {name: 'ɵɵvalidateIframeAttribute', moduleName: CORE};
339341
}

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

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import * as core from '../../core';
1313
import {AST, ParsedEvent, ParsedEventType, ParsedProperty} from '../../expression_parser/ast';
1414
import * as o from '../../output/output_ast';
1515
import {ParseError, ParseSourceSpan, sanitizeIdentifier} from '../../parse_util';
16+
import {isIframeSecuritySensitiveAttr} from '../../schema/dom_security_schema';
1617
import {CssSelector, SelectorMatcher} from '../../selector';
1718
import {ShadowCss} from '../../shadow_css';
1819
import {CONTENT_ATTR, HOST_ATTR} from '../../style_compiler';
@@ -561,6 +562,19 @@ function createHostBindingsFunction(
561562
const instructionParams = [o.literal(bindingName), bindingExpr.currValExpr];
562563
if (sanitizerFn) {
563564
instructionParams.push(sanitizerFn);
565+
} else {
566+
// If there was no sanitization function found based on the security context
567+
// of an attribute/property binding - check whether this attribute/property is
568+
// one of the security-sensitive <iframe> attributes.
569+
// Note: for host bindings defined on a directive, we do not try to find all
570+
// possible places where it can be matched, so we can not determine whether
571+
// the host element is an <iframe>. In this case, if an attribute/binding
572+
// name is in the `IFRAME_SECURITY_SENSITIVE_ATTRS` set - append a validation
573+
// function, which would be invoked at runtime and would have access to the
574+
// underlying DOM element, check if it's an <iframe> and if so - runs extra checks.
575+
if (isIframeSecuritySensitiveAttr(bindingName)) {
576+
instructionParams.push(o.importExpr(R3.validateIframeAttribute));
577+
}
564578
}
565579

566580
updateStatements.push(...bindingExpr.stmts);

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

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import {mapLiteral} from '../../output/map_util';
2424
import * as o from '../../output/output_ast';
2525
import {ParseError, ParseSourceSpan, sanitizeIdentifier} from '../../parse_util';
2626
import {DomElementSchemaRegistry} from '../../schema/dom_element_schema_registry';
27+
import {isIframeSecuritySensitiveAttr} from '../../schema/dom_security_schema';
2728
import {isTrustedTypesSink} from '../../schema/trusted_types_sinks';
2829
import {CssSelector, SelectorMatcher} from '../../selector';
2930
import {BindingParser} from '../../template_parser/binding_parser';
@@ -782,8 +783,19 @@ export class TemplateDefinitionBuilder implements t.Visitor<void>, LocalResolver
782783
const params: any[] = [];
783784
const [attrNamespace, attrName] = splitNsName(input.name);
784785
const isAttributeBinding = inputType === BindingType.Attribute;
785-
const sanitizationRef = resolveSanitizationFn(input.securityContext, isAttributeBinding);
786-
if (sanitizationRef) params.push(sanitizationRef);
786+
let sanitizationRef = resolveSanitizationFn(input.securityContext, isAttributeBinding);
787+
if (!sanitizationRef) {
788+
// If there was no sanitization function found based on the security context
789+
// of an attribute/property - check whether this attribute/property is
790+
// one of the security-sensitive <iframe> attributes (and that the current
791+
// element is actually an <iframe>).
792+
if (isIframeElement(element.name) && isIframeSecuritySensitiveAttr(input.name)) {
793+
sanitizationRef = o.importExpr(R3.validateIframeAttribute);
794+
}
795+
}
796+
if (sanitizationRef) {
797+
params.push(sanitizationRef);
798+
}
787799
if (attrNamespace) {
788800
const namespaceLiteral = o.literal(attrNamespace);
789801

@@ -2278,6 +2290,10 @@ function isTextNode(node: t.Node): boolean {
22782290
return node instanceof t.Text || node instanceof t.BoundText || node instanceof t.Icu;
22792291
}
22802292

2293+
function isIframeElement(tagName: string): boolean {
2294+
return tagName.toLowerCase() === 'iframe';
2295+
}
2296+
22812297
function hasTextChildrenOnly(children: t.Node[]): boolean {
22822298
return children.every(isTextNode);
22832299
}

‎packages/compiler/src/schema/dom_security_schema.ts‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,3 +62,25 @@ export function SECURITY_SCHEMA(): {[k: string]: SecurityContext} {
6262
function registerContext(ctx: SecurityContext, specs: string[]) {
6363
for (const spec of specs) _SECURITY_SCHEMA[spec.toLowerCase()] = ctx;
6464
}
65+
66+
/**
67+
* The set of security-sensitive attributes of an `<iframe>` that *must* be
68+
* applied as a static attribute only. This ensures that all security-sensitive
69+
* attributes are taken into account while creating an instance of an `<iframe>`
70+
* at runtime.
71+
*
72+
* Note: avoid using this set directly, use the `isIframeSecuritySensitiveAttr` function
73+
* in the code instead.
74+
*/
75+
export const IFRAME_SECURITY_SENSITIVE_ATTRS =
76+
new Set(['sandbox', 'allow', 'allowfullscreen', 'referrerpolicy', 'csp', 'fetchpriority']);
77+
78+
/**
79+
* Checks whether a given attribute name might represent a security-sensitive
80+
* attribute of an <iframe>.
81+
*/
82+
export function isIframeSecuritySensitiveAttr(attrName: string): boolean {
83+
// The `setAttribute` DOM API is case-insensitive, so we lowercase the value
84+
// before checking it against a known security-sensitive attributes.
85+
return IFRAME_SECURITY_SENSITIVE_ATTRS.has(attrName.toLowerCase());
86+
}

‎packages/compiler/test/security_spec.ts‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
/**
2+
* @license
3+
* Copyright Google LLC All Rights Reserved.
4+
*
5+
* Use of this source code is governed by an MIT-style license that can be
6+
* found in the LICENSE file at https://angular.io/license
7+
*/
8+
9+
import {IFRAME_SECURITY_SENSITIVE_ATTRS, SECURITY_SCHEMA} from '../src/schema/dom_security_schema';
10+
11+
12+
describe('security-related tests', () => {
13+
it('should have no overlap between `IFRAME_SECURITY_SENSITIVE_ATTRS` and `SECURITY_SCHEMA`',
14+
() => {
15+
// The `IFRAME_SECURITY_SENSITIVE_ATTRS` and `SECURITY_SCHEMA` tokens configure sanitization
16+
// and validation rules and used to pick the right sanitizer function.
17+
// This test verifies that there is no overlap between two sets of rules to flag
18+
// a situation when 2 sanitizer functions may be needed at the same time (in which
19+
// case, compiler logic should be extended to support that).
20+
const schema = new Set();
21+
Object.keys(SECURITY_SCHEMA()).forEach((key: string) => schema.add(key.toLowerCase()));
22+
let hasOverlap = false;
23+
IFRAME_SECURITY_SENSITIVE_ATTRS.forEach(attr => {
24+
if (schema.has('*|' + attr) || schema.has('iframe|' + attr)) {
25+
hasOverlap = true;
26+
}
27+
});
28+
expect(hasOverlap).toBeFalse();
29+
});
30+
});

‎packages/core/src/core_render3_private_export.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -303,6 +303,9 @@ export {
303303
ɵɵtrustConstantHtml,
304304
ɵɵtrustConstantResourceUrl,
305305
} from './sanitization/sanitization';
306+
export {
307+
ɵɵvalidateIframeAttribute,
308+
} from './sanitization/iframe_attrs_validation';
306309
export {
307310
noSideEffects as ɵnoSideEffects,
308311
} from './util/closure';

‎packages/core/src/render3/error_code.ts‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ export const enum RuntimeErrorCode {
2424
PIPE_NOT_FOUND = '302',
2525
UNKNOWN_BINDING = '303',
2626
UNKNOWN_ELEMENT = '304',
27-
TEMPLATE_STRUCTURE_ERROR = '305'
27+
TEMPLATE_STRUCTURE_ERROR = '305',
2828

2929
// Styling Errors
3030

@@ -33,6 +33,9 @@ export const enum RuntimeErrorCode {
3333
// i18n Errors
3434

3535
// Compilation Errors
36+
37+
// Other
38+
UNSAFE_IFRAME_ATTRS = '910',
3639
}
3740

3841
export class RuntimeError extends Error {

‎packages/core/src/render3/instructions/element_validation.ts‎

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
/**
2+
* @license
3+
* Copyright Google LLC All Rights Reserved.
4+
*
5+
* Use of this source code is governed by an MIT-style license that can be
6+
* found in the LICENSE file at https://angular.io/license
7+
*/
8+
9+
import {throwError} from '../../util/assert';
10+
import {getComponentDef} from '../definition';
11+
import {ComponentDef} from '../interfaces/definition';
12+
import {CONTEXT, DECLARATION_COMPONENT_VIEW, LView} from '../interfaces/view';
13+
14+
/**
15+
* WARNING: this is a **dev-mode only** function (thus should always be guarded by the `ngDevMode`)
16+
* and must **not** be used in production bundles. The function makes megamorphic reads, which might
17+
* be too slow for production mode and also it relies on the constructor function being available.
18+
*
19+
* Gets a reference to the host component def (where a current component is declared).
20+
*
21+
* @param lView An `LView` that represents a current component that is being rendered.
22+
*/
23+
function getDeclarationComponentDef(lView: LView): ComponentDef<unknown>|null {
24+
!ngDevMode && throwError('Must never be called in production mode');
25+
26+
const declarationLView = lView[DECLARATION_COMPONENT_VIEW] as LView;
27+
const context = declarationLView[CONTEXT];
28+
29+
// Unable to obtain a context.
30+
if (!context) return null;
31+
32+
return context.constructor ? getComponentDef(context.constructor) : null;
33+
}
34+
35+
/**
36+
* WARNING: this is a **dev-mode only** function (thus should always be guarded by the `ngDevMode`)
37+
* and must **not** be used in production bundles. The function makes megamorphic reads, which might
38+
* be too slow for production mode.
39+
*
40+
* Constructs a string describing the location of the host component template. The function is used
41+
* in dev mode to produce error messages.
42+
*
43+
* @param lView An `LView` that represents a current component that is being rendered.
44+
*/
45+
export function getTemplateLocationDetails(lView: LView): string {
46+
!ngDevMode && throwError('Must never be called in production mode');
47+
48+
const hostComponentDef = getDeclarationComponentDef(lView);
49+
const componentClassName = hostComponentDef?.type?.name;
50+
return componentClassName ? ` (used in the '${componentClassName}' component template)` : '';
51+
}

‎packages/core/src/render3/jit/environment.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
import {forwardRef, resolveForwardRef} from '../../di/forward_ref';
1010
import {ɵɵinject, ɵɵinvalidFactoryDep} from '../../di/injector_compatibility';
1111
import {ɵɵdefineInjectable, ɵɵdefineInjector} from '../../di/interface/defs';
12+
import * as iframe_attrs_validation from '../../sanitization/iframe_attrs_validation';
1213
import * as sanitization from '../../sanitization/sanitization';
1314
import * as r3 from '../index';
1415

@@ -164,6 +165,7 @@ export const angularCoreEnv: {[name: string]: Function} =
164165
'ɵɵsanitizeUrlOrResourceUrl': sanitization.ɵɵsanitizeUrlOrResourceUrl,
165166
'ɵɵtrustConstantHtml': sanitization.ɵɵtrustConstantHtml,
166167
'ɵɵtrustConstantResourceUrl': sanitization.ɵɵtrustConstantResourceUrl,
168+
'ɵɵvalidateIframeAttribute': iframe_attrs_validation.ɵɵvalidateIframeAttribute,
167169

168170
'forwardRef': forwardRef,
169171
'resolveForwardRef': resolveForwardRef,

0 commit comments

Comments
 (0)