Visitar URL original
fix(core): sanitize host bindings on concrete hosts (#69558) · angular/angular@23cf1a8 · GitHub
Skip to content

Commit 23cf1a8

Browse files
SkyZeroZxalxhub
SkyZeroZx
authored and
alxhub
committed
fix(core): sanitize host bindings on concrete hosts (#69558)
Host binding sanitization previously used the declaring directive or component selector to choose a compile-time security context. The same host binding can execute on a different concrete element through hostDirectives, inherited host bindings, dynamic directives, or createComponent hostElement usage. Compute host binding security contexts against possible concrete hosts and defer URL versus ResourceURL selection to runtime when necessary. Resolve dynamic root host TNodes to their native tag before sanitizer and security-sensitive attribute checks. Fixes #69550 PR Close #69558
1 parent 5d5b2ea commit 23cf1a8

13 files changed

Lines changed: 747 additions & 53 deletions

File tree

‎packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/host_bindings/GOLDEN_PARTIAL.js‎

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -981,6 +981,48 @@ i0.ɵɵngDeclareClassMetadata({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDE
981981
},
982982
}]
983983
}] });
984+
export class HostBindingCustomSrcdocDir {
985+
evil = 'evil';
986+
static ɵfac = i0.ɵɵngDeclareFactory({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: HostBindingCustomSrcdocDir, deps: [], target: i0.ɵɵFactoryTarget.Directive });
987+
static ɵdir = i0.ɵɵngDeclareDirective({ minVersion: "14.0.0", version: "0.0.0-PLACEHOLDER", type: HostBindingCustomSrcdocDir, isStandalone: true, selector: "safe-srcdoc-carrier", host: { properties: { "attr.srcdoc": "evil" } }, ngImport: i0 });
988+
}
989+
i0.ɵɵngDeclareClassMetadata({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: HostBindingCustomSrcdocDir, decorators: [{
990+
type: Directive,
991+
args: [{
992+
selector: 'safe-srcdoc-carrier',
993+
host: {
994+
'[attr.srcdoc]': 'evil',
995+
},
996+
}]
997+
}] });
998+
export class HostBindingCustomSrcDir {
999+
evil = 'evil';
1000+
static ɵfac = i0.ɵɵngDeclareFactory({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: HostBindingCustomSrcDir, deps: [], target: i0.ɵɵFactoryTarget.Directive });
1001+
static ɵdir = i0.ɵɵngDeclareDirective({ minVersion: "14.0.0", version: "0.0.0-PLACEHOLDER", type: HostBindingCustomSrcDir, isStandalone: true, selector: "safe-src-carrier", host: { properties: { "attr.src": "evil" } }, ngImport: i0 });
1002+
}
1003+
i0.ɵɵngDeclareClassMetadata({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: HostBindingCustomSrcDir, decorators: [{
1004+
type: Directive,
1005+
args: [{
1006+
selector: 'safe-src-carrier',
1007+
host: {
1008+
'[attr.src]': 'evil',
1009+
},
1010+
}]
1011+
}] });
1012+
export class HostBindingCustomDataDir {
1013+
evil = 'evil';
1014+
static ɵfac = i0.ɵɵngDeclareFactory({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: HostBindingCustomDataDir, deps: [], target: i0.ɵɵFactoryTarget.Directive });
1015+
static ɵdir = i0.ɵɵngDeclareDirective({ minVersion: "14.0.0", version: "0.0.0-PLACEHOLDER", type: HostBindingCustomDataDir, isStandalone: true, selector: "safe-data-carrier", host: { properties: { "attr.data": "evil" } }, ngImport: i0 });
1016+
}
1017+
i0.ɵɵngDeclareClassMetadata({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: HostBindingCustomDataDir, decorators: [{
1018+
type: Directive,
1019+
args: [{
1020+
selector: 'safe-data-carrier',
1021+
host: {
1022+
'[attr.data]': 'evil',
1023+
},
1024+
}]
1025+
}] });
9841026

9851027
/****************************************************************************************************
9861028
* PARTIAL FILE: sanitization.d.ts
@@ -1008,6 +1050,21 @@ export declare class HostBindingSvgAnimateDir {
10081050
static ɵfac: i0.ɵɵFactoryDeclaration<HostBindingSvgAnimateDir, never>;
10091051
static ɵdir: i0.ɵɵDirectiveDeclaration<HostBindingSvgAnimateDir, "animateMotion[hostBindingSvgAnimateDir]", never, {}, {}, never, never, true, never>;
10101052
}
1053+
export declare class HostBindingCustomSrcdocDir {
1054+
evil: string;
1055+
static ɵfac: i0.ɵɵFactoryDeclaration<HostBindingCustomSrcdocDir, never>;
1056+
static ɵdir: i0.ɵɵDirectiveDeclaration<HostBindingCustomSrcdocDir, "safe-srcdoc-carrier", never, {}, {}, never, never, true, never>;
1057+
}
1058+
export declare class HostBindingCustomSrcDir {
1059+
evil: string;
1060+
static ɵfac: i0.ɵɵFactoryDeclaration<HostBindingCustomSrcDir, never>;
1061+
static ɵdir: i0.ɵɵDirectiveDeclaration<HostBindingCustomSrcDir, "safe-src-carrier", never, {}, {}, never, never, true, never>;
1062+
}
1063+
export declare class HostBindingCustomDataDir {
1064+
evil: string;
1065+
static ɵfac: i0.ɵɵFactoryDeclaration<HostBindingCustomDataDir, never>;
1066+
static ɵdir: i0.ɵɵDirectiveDeclaration<HostBindingCustomDataDir, "safe-data-carrier", never, {}, {}, never, never, true, never>;
1067+
}
10111068

10121069
/****************************************************************************************************
10131070
* PARTIAL FILE: security_sensitive_constant_attributes.js

‎packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/host_bindings/sanitization.js‎

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,21 +1,21 @@
11
hostBindings: function HostBindingLinkDir_HostBindings(rf, ctx) {
22
if (rf & 2) {
3-
$r3$.ɵɵdomProperty("innerHTML", ctx.evil, $r3$.ɵɵsanitizeHtml)("href", ctx.evil, $r3$.ɵɵsanitizeUrl);
3+
$r3$.ɵɵdomProperty("innerHTML", ctx.evil, $r3$.ɵɵsanitizeHtml)("href", ctx.evil, $r3$.ɵɵsanitizeUrlOrResourceUrl);
44
$r3$.ɵɵattribute("style", ctx.evil, $r3$.ɵɵsanitizeStyle);
55
}
66
}
77
…
88
hostBindings: function HostBindingImageDir_HostBindings(rf, ctx) {
99
if (rf & 2) {
10-
i0.ɵɵdomProperty("innerHTML", ctx.evil, i0.ɵɵsanitizeHtml)("src", ctx.nonEvil, i0.ɵɵsanitizeUrl);
10+
i0.ɵɵdomProperty("innerHTML", ctx.evil, i0.ɵɵsanitizeHtml)("src", ctx.nonEvil, i0.ɵɵsanitizeUrlOrResourceUrl);
1111
i0.ɵɵattribute("style", ctx.evil, i0.ɵɵsanitizeStyle);
1212
}
1313
}
1414
…
1515
hostBindings: function HostBindingIframeDir_HostBindings(rf, ctx) {
1616
if (rf & 2) {
17-
$r3$.ɵɵdomProperty("innerHTML", ctx.evil, $r3$.ɵɵsanitizeHtml)("src", ctx.evil, i0.ɵɵsanitizeResourceUrl)("sandbox", ctx.evil, $r3$.ɵɵvalidateAttribute);
18-
$r3$.ɵɵattribute("style", ctx.evil, $r3$.ɵɵsanitizeStyle)("attributeName", ctx.nonEvil);
17+
$r3$.ɵɵdomProperty("innerHTML", ctx.evil, $r3$.ɵɵsanitizeHtml)("src", ctx.evil, i0.ɵɵsanitizeUrlOrResourceUrl)("sandbox", ctx.evil, $r3$.ɵɵvalidateAttribute);
18+
$r3$.ɵɵattribute("style", ctx.evil, $r3$.ɵɵsanitizeStyle)("attributeName", ctx.nonEvil, i0.ɵɵvalidateAttribute);
1919
}
2020
}
2121
…
@@ -24,3 +24,21 @@ hostBindings: function HostBindingSvgAnimateDir_HostBindings(rf, ctx) {
2424
i0.ɵɵattribute("attributeName", ctx.evil, i0.ɵɵvalidateAttribute);
2525
}
2626
}
27+
…
28+
hostBindings: function HostBindingCustomSrcdocDir_HostBindings(rf, ctx) {
29+
if (rf & 2) {
30+
i0.ɵɵattribute("srcdoc", ctx.evil, i0.ɵɵsanitizeHtml);
31+
}
32+
}
33+
…
34+
hostBindings: function HostBindingCustomSrcDir_HostBindings(rf, ctx) {
35+
if (rf & 2) {
36+
i0.ɵɵattribute("src", ctx.evil, i0.ɵɵsanitizeUrlOrResourceUrl);
37+
}
38+
}
39+
…
40+
hostBindings: function HostBindingCustomDataDir_HostBindings(rf, ctx) {
41+
if (rf & 2) {
42+
i0.ɵɵattribute("data", ctx.evil, i0.ɵɵsanitizeUrlOrResourceUrl);
43+
}
44+
}

‎packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/host_bindings/sanitization.ts‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,3 +49,33 @@ export class HostBindingIframeDir {
4949
export class HostBindingSvgAnimateDir {
5050
evil = 'evil';
5151
}
52+
53+
@Directive({
54+
selector: 'safe-srcdoc-carrier',
55+
host: {
56+
'[attr.srcdoc]': 'evil',
57+
},
58+
})
59+
export class HostBindingCustomSrcdocDir {
60+
evil = 'evil';
61+
}
62+
63+
@Directive({
64+
selector: 'safe-src-carrier',
65+
host: {
66+
'[attr.src]': 'evil',
67+
},
68+
})
69+
export class HostBindingCustomSrcDir {
70+
evil = 'evil';
71+
}
72+
73+
@Directive({
74+
selector: 'safe-data-carrier',
75+
host: {
76+
'[attr.data]': 'evil',
77+
},
78+
})
79+
export class HostBindingCustomDataDir {
80+
evil = 'evil';
81+
}

‎packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/host_bindings/sanitization_isolated.golden.d.ts‎

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,4 +21,18 @@ export declare class HostBindingSvgAnimateDir {
2121
static ɵfac: i0.ɵɵFactoryDeclaration<HostBindingSvgAnimateDir, never>;
2222
static ɵdir: i0.ɵɵDirectiveDeclaration<HostBindingSvgAnimateDir, "animateMotion[hostBindingSvgAnimateDir]", never, {}, {}, never, never, true, never>;
2323
}
24-
24+
export declare class HostBindingCustomSrcdocDir {
25+
evil: string;
26+
static ɵfac: i0.ɵɵFactoryDeclaration<HostBindingCustomSrcdocDir, never>;
27+
static ɵdir: i0.ɵɵDirectiveDeclaration<HostBindingCustomSrcdocDir, "safe-srcdoc-carrier", never, {}, {}, never, never, true, never>;
28+
}
29+
export declare class HostBindingCustomSrcDir {
30+
evil: string;
31+
static ɵfac: i0.ɵɵFactoryDeclaration<HostBindingCustomSrcDir, never>;
32+
static ɵdir: i0.ɵɵDirectiveDeclaration<HostBindingCustomSrcDir, "safe-src-carrier", never, {}, {}, never, never, true, never>;
33+
}
34+
export declare class HostBindingCustomDataDir {
35+
evil: string;
36+
static ɵfac: i0.ɵɵFactoryDeclaration<HostBindingCustomDataDir, never>;
37+
static ɵdir: i0.ɵɵDirectiveDeclaration<HostBindingCustomDataDir, "safe-data-carrier", never, {}, {}, never, never, true, never>;
38+
}

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

Lines changed: 37 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8614,7 +8614,7 @@ runInEachFileSystem((os: string) => {
86148614
hostVars: 6,
86158615
hostBindings: function UnsafeAttrsDirective_HostBindings(rf, ctx) {
86168616
if (rf & 2) {
8617-
i0.ɵɵattribute("href", ctx.attrHref, i0.ɵɵsanitizeUrlOrResourceUrl)("src", ctx.attrSrc, i0.ɵɵsanitizeUrlOrResourceUrl)("action", ctx.attrAction, i0.ɵɵsanitizeUrl)("profile", ctx.attrProfile)("innerHTML", ctx.attrInnerHTML, i0.ɵɵsanitizeHtml)("title", ctx.attrSafeTitle);
8617+
i0.ɵɵattribute("href", ctx.attrHref, i0.ɵɵsanitizeUrlOrResourceUrl)("src", ctx.attrSrc, i0.ɵɵsanitizeUrlOrResourceUrl)("action", ctx.attrAction, i0.ɵɵsanitizeUrlOrResourceUrl)("profile", ctx.attrProfile)("innerHTML", ctx.attrInnerHTML, i0.ɵɵsanitizeHtml)("title", ctx.attrSafeTitle);
86188618
}
86198619
}
86208620
`;
@@ -8660,14 +8660,14 @@ runInEachFileSystem((os: string) => {
86608660
hostVars: 3,
86618661
hostBindings: function UnsafePropsDirective_HostBindings(rf, ctx) {
86628662
if (rf & 2) {
8663-
i0.ɵɵdomProperty("href", ctx.propHref, i0.ɵɵsanitizeUrl)("innerHTML", ctx.propInnerHTML, i0.ɵɵsanitizeHtml)("title", ctx.propSafeTitle);
8663+
i0.ɵɵdomProperty("href", ctx.propHref, i0.ɵɵsanitizeUrlOrResourceUrl)("innerHTML", ctx.propInnerHTML, i0.ɵɵsanitizeHtml)("title", ctx.propSafeTitle);
86648664
}
86658665
}
86668666
`;
86678667
expect(trim(jsContents)).toContain(trim(hostBindingsFn));
86688668
});
86698669

8670-
it('should not generate sanitizers for URL properties in hostBindings fn in Component', () => {
8670+
it('should generate concrete-host URL sanitizers in hostBindings fn in Component', () => {
86718671
env.write(
86728672
`test.ts`,
86738673
`
@@ -8700,8 +8700,40 @@ runInEachFileSystem((os: string) => {
87008700
hostVars: 5,
87018701
hostBindings: function FooCmp_HostBindings(rf, ctx) {
87028702
if (rf & 2) {
8703-
i0.ɵɵdomProperty("href", ctx.hrefProp, i0.ɵɵsanitizeUrl)("title", ctx.titleProp);
8704-
i0.ɵɵattribute("src", ctx.srcAttr)("href", ctx.hrefAttr, i0.ɵɵsanitizeUrl)("title", ctx.titleAttr);
8703+
i0.ɵɵdomProperty("href", ctx.hrefProp, i0.ɵɵsanitizeUrlOrResourceUrl)("title", ctx.titleProp);
8704+
i0.ɵɵattribute("src", ctx.srcAttr, i0.ɵɵsanitizeUrlOrResourceUrl)("href", ctx.hrefAttr, i0.ɵɵsanitizeUrlOrResourceUrl)("title", ctx.titleAttr);
8705+
}
8706+
}
8707+
`;
8708+
expect(trim(jsContents)).toContain(trim(hostBindingsFn));
8709+
});
8710+
8711+
it('should generate sanitizers for pure :not selector host bindings', () => {
8712+
env.write(
8713+
`test.ts`,
8714+
`
8715+
import {Component} from '@angular/core';
8716+
8717+
@Component({
8718+
selector: ':not(iframe)',
8719+
template: '',
8720+
host: {
8721+
'[attr.srcdoc]': 'srcdoc',
8722+
}
8723+
})
8724+
class FooCmp {
8725+
srcdoc: any;
8726+
}
8727+
`,
8728+
);
8729+
8730+
env.driveMain();
8731+
const jsContents = env.getContents('test.js');
8732+
const hostBindingsFn = `
8733+
hostVars: 1,
8734+
hostBindings: function FooCmp_HostBindings(rf, ctx) {
8735+
if (rf & 2) {
8736+
i0.ɵɵattribute("srcdoc", ctx.srcdoc, i0.ɵɵsanitizeHtml);
87058737
}
87068738
}
87078739
`;

‎packages/compiler/src/template/pipeline/src/ingest.ts‎

Lines changed: 49 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ import {
2121
} from '../../../render3/view/api';
2222
import {icuFromI18nMessage} from '../../../render3/view/i18n/util';
2323
import {DomElementSchemaRegistry} from '../../../schema/dom_element_schema_registry';
24-
import {BindingParser} from '../../../template_parser/binding_parser';
24+
import {BindingParser, calcPossibleSecurityContexts} from '../../../template_parser/binding_parser';
2525
import * as ir from '../ir';
2626

2727
import {
@@ -125,19 +125,21 @@ export function ingestHostBinding(
125125
if (property.isAnimation) {
126126
bindingKind = ir.BindingKind.Animation;
127127
}
128-
const securityContexts = bindingParser
129-
.calcPossibleSecurityContexts(
130-
input.componentSelector,
131-
property.name,
132-
bindingKind === ir.BindingKind.Attribute,
133-
)
134-
.filter((context) => context !== SecurityContext.NONE);
128+
const securityContexts = calcHostBindingSecurityContexts(
129+
bindingParser,
130+
input.componentSelector,
131+
property.name,
132+
bindingKind === ir.BindingKind.Attribute,
133+
);
135134
ingestDomProperty(job, property, bindingKind, securityContexts);
136135
}
137136
for (const [name, expr] of Object.entries(input.attributes) ?? []) {
138-
const securityContexts = bindingParser
139-
.calcPossibleSecurityContexts(input.componentSelector, name, true)
140-
.filter((context) => context !== SecurityContext.NONE);
137+
const securityContexts = calcHostBindingSecurityContexts(
138+
bindingParser,
139+
input.componentSelector,
140+
name,
141+
true,
142+
);
141143
ingestHostAttribute(job, name, expr, securityContexts);
142144
}
143145
for (const event of input.events ?? []) {
@@ -146,6 +148,42 @@ export function ingestHostBinding(
146148
return job;
147149
}
148150

151+
function calcHostBindingSecurityContexts(
152+
bindingParser: BindingParser,
153+
selector: string,
154+
name: string,
155+
isAttribute: boolean,
156+
): SecurityContext[] {
157+
const declaringSelectorContexts = bindingParser.calcPossibleSecurityContexts(
158+
selector,
159+
name,
160+
isAttribute,
161+
);
162+
const concreteHostContexts = calcPossibleSecurityContexts(
163+
domSchema,
164+
null,
165+
domSchema.getMappedPropName(name),
166+
isAttribute,
167+
);
168+
const concreteHostNonNoneContexts = concreteHostContexts.filter(
169+
(context) => context !== SecurityContext.NONE,
170+
);
171+
const concreteHostNonNoneCount = concreteHostNonNoneContexts.length;
172+
const hasConcreteHostNoneContext = concreteHostNonNoneCount !== concreteHostContexts.length;
173+
174+
// Host bindings can run against a concrete host whose element name differs from the declaring
175+
// selector, including dynamic root components whose TNode name is `#host`.
176+
if (hasConcreteHostNoneContext && concreteHostNonNoneCount > 0) {
177+
return concreteHostContexts;
178+
}
179+
180+
if (concreteHostNonNoneContexts.some((context) => !declaringSelectorContexts.includes(context))) {
181+
return concreteHostContexts;
182+
}
183+
184+
return declaringSelectorContexts.filter((context) => context !== SecurityContext.NONE);
185+
}
186+
149187
// TODO: We should refactor the parser to use the same types and structures for host bindings as
150188
// with ordinary components. This would allow us to share a lot more ingestion code.
151189
export function ingestDomProperty(

0 commit comments

Comments
 (0)