Skip to content

Commit 2f96c80

Browse files
SkyZeroZxSkyZeroZx
authored
fix(core): sanitize host bindings on concrete hosts
Compute host binding security contexts against concrete hosts, including host directives, inheritance, dynamic directives, and createComponent hostElement usage.
1 parent 72a8512 commit 2f96c80

19 files changed

Lines changed: 889 additions & 186 deletions

File tree

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

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -933,6 +933,54 @@ i0.ɵɵngDeclareClassMetadata({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDE
933933
},
934934
}]
935935
}] });
936+
export class HostBindingCustomSrcdocDir {
937+
constructor() {
938+
this.evil = 'evil';
939+
}
940+
}
941+
HostBindingCustomSrcdocDir.ɵfac = i0.ɵɵngDeclareFactory({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: HostBindingCustomSrcdocDir, deps: [], target: i0.ɵɵFactoryTarget.Directive });
942+
HostBindingCustomSrcdocDir.ɵ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 });
943+
i0.ɵɵngDeclareClassMetadata({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: HostBindingCustomSrcdocDir, decorators: [{
944+
type: Directive,
945+
args: [{
946+
selector: 'safe-srcdoc-carrier',
947+
host: {
948+
'[attr.srcdoc]': 'evil',
949+
},
950+
}]
951+
}] });
952+
export class HostBindingCustomSrcDir {
953+
constructor() {
954+
this.evil = 'evil';
955+
}
956+
}
957+
HostBindingCustomSrcDir.ɵfac = i0.ɵɵngDeclareFactory({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: HostBindingCustomSrcDir, deps: [], target: i0.ɵɵFactoryTarget.Directive });
958+
HostBindingCustomSrcDir.ɵ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 });
959+
i0.ɵɵngDeclareClassMetadata({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: HostBindingCustomSrcDir, decorators: [{
960+
type: Directive,
961+
args: [{
962+
selector: 'safe-src-carrier',
963+
host: {
964+
'[attr.src]': 'evil',
965+
},
966+
}]
967+
}] });
968+
export class HostBindingCustomDataDir {
969+
constructor() {
970+
this.evil = 'evil';
971+
}
972+
}
973+
HostBindingCustomDataDir.ɵfac = i0.ɵɵngDeclareFactory({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: HostBindingCustomDataDir, deps: [], target: i0.ɵɵFactoryTarget.Directive });
974+
HostBindingCustomDataDir.ɵ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 });
975+
i0.ɵɵngDeclareClassMetadata({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: HostBindingCustomDataDir, decorators: [{
976+
type: Directive,
977+
args: [{
978+
selector: 'safe-data-carrier',
979+
host: {
980+
'[attr.data]': 'evil',
981+
},
982+
}]
983+
}] });
936984

937985
/****************************************************************************************************
938986
* PARTIAL FILE: sanitization.d.ts
@@ -954,6 +1002,21 @@ export declare class HostBindingSvgAnimateDir {
9541002
static ɵfac: i0.ɵɵFactoryDeclaration<HostBindingSvgAnimateDir, never>;
9551003
static ɵdir: i0.ɵɵDirectiveDeclaration<HostBindingSvgAnimateDir, "animateMotion[hostBindingSvgAnimateDir]", never, {}, {}, never, never, true, never>;
9561004
}
1005+
export declare class HostBindingCustomSrcdocDir {
1006+
evil: string;
1007+
static ɵfac: i0.ɵɵFactoryDeclaration<HostBindingCustomSrcdocDir, never>;
1008+
static ɵdir: i0.ɵɵDirectiveDeclaration<HostBindingCustomSrcdocDir, "safe-srcdoc-carrier", never, {}, {}, never, never, true, never>;
1009+
}
1010+
export declare class HostBindingCustomSrcDir {
1011+
evil: string;
1012+
static ɵfac: i0.ɵɵFactoryDeclaration<HostBindingCustomSrcDir, never>;
1013+
static ɵdir: i0.ɵɵDirectiveDeclaration<HostBindingCustomSrcDir, "safe-src-carrier", never, {}, {}, never, never, true, never>;
1014+
}
1015+
export declare class HostBindingCustomDataDir {
1016+
evil: string;
1017+
static ɵfac: i0.ɵɵFactoryDeclaration<HostBindingCustomDataDir, never>;
1018+
static ɵdir: i0.ɵɵDirectiveDeclaration<HostBindingCustomDataDir, "safe-data-carrier", never, {}, {}, never, never, true, never>;
1019+
}
9571020

9581021
/****************************************************************************************************
9591022
* PARTIAL FILE: security_sensitive_constant_attributes.js

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

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ hostBindings: function HostBindingDir_HostBindings(rf, ctx) {
77
…
88
hostBindings: function HostBindingDir2_HostBindings(rf, ctx) {
99
if (rf & 2) {
10-
i0.ɵɵdomProperty("innerHTML", ctx.evil, i0.ɵɵsanitizeHtml)("href", ctx.evil, i0.ɵɵsanitizeUrl)("src", ctx.evil)("sandbox", ctx.evil);
10+
i0.ɵɵdomProperty("innerHTML", ctx.evil, i0.ɵɵsanitizeHtml)("href", ctx.evil, i0.ɵɵsanitizeUrlOrResourceUrl)("src", ctx.evil, i0.ɵɵsanitizeUrlOrResourceUrl)("sandbox", ctx.evil, i0.ɵɵvalidateAttribute);
1111
i0.ɵɵattribute("style", ctx.evil, i0.ɵɵsanitizeStyle);
1212
}
1313
}
@@ -16,4 +16,22 @@ hostBindings: function HostBindingSvgAnimateDir_HostBindings(rf, ctx) {
1616
if (rf & 2) {
1717
i0.ɵɵattribute("attributeName", ctx.evil, i0.ɵɵvalidateAttribute);
1818
}
19-
}
19+
}
20+
…
21+
hostBindings: function HostBindingCustomSrcdocDir_HostBindings(rf, ctx) {
22+
if (rf & 2) {
23+
i0.ɵɵattribute("srcdoc", ctx.evil, i0.ɵɵsanitizeHtml);
24+
}
25+
}
26+
…
27+
hostBindings: function HostBindingCustomSrcDir_HostBindings(rf, ctx) {
28+
if (rf & 2) {
29+
i0.ɵɵattribute("src", ctx.evil, i0.ɵɵsanitizeUrlOrResourceUrl);
30+
}
31+
}
32+
…
33+
hostBindings: function HostBindingCustomDataDir_HostBindings(rf, ctx) {
34+
if (rf & 2) {
35+
i0.ɵɵattribute("data", ctx.evil, i0.ɵɵsanitizeUrlOrResourceUrl);
36+
}
37+
}

‎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
@@ -39,3 +39,33 @@ export class HostBindingDir2 {
3939
export class HostBindingSvgAnimateDir {
4040
evil = 'evil';
4141
}
42+
43+
@Directive({
44+
selector: 'safe-srcdoc-carrier',
45+
host: {
46+
'[attr.srcdoc]': 'evil',
47+
},
48+
})
49+
export class HostBindingCustomSrcdocDir {
50+
evil = 'evil';
51+
}
52+
53+
@Directive({
54+
selector: 'safe-src-carrier',
55+
host: {
56+
'[attr.src]': 'evil',
57+
},
58+
})
59+
export class HostBindingCustomSrcDir {
60+
evil = 'evil';
61+
}
62+
63+
@Directive({
64+
selector: 'safe-data-carrier',
65+
host: {
66+
'[attr.data]': 'evil',
67+
},
68+
})
69+
export class HostBindingCustomDataDir {
70+
evil = 'evil';
71+
}

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

Lines changed: 37 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8461,7 +8461,7 @@ runInEachFileSystem((os: string) => {
84618461
hostVars: 5,
84628462
hostBindings: function UnsafeAttrsDirective_HostBindings(rf, ctx) {
84638463
if (rf & 2) {
8464-
i0.ɵɵattribute("href", ctx.attrHref, i0.ɵɵsanitizeUrlOrResourceUrl)("src", ctx.attrSrc, i0.ɵɵsanitizeUrlOrResourceUrl)("action", ctx.attrAction, i0.ɵɵsanitizeUrl)("innerHTML", ctx.attrInnerHTML, i0.ɵɵsanitizeHtml)("title", ctx.attrSafeTitle);
8464+
i0.ɵɵattribute("href", ctx.attrHref, i0.ɵɵsanitizeUrlOrResourceUrl)("src", ctx.attrSrc, i0.ɵɵsanitizeUrlOrResourceUrl)("action", ctx.attrAction, i0.ɵɵsanitizeUrlOrResourceUrl)("innerHTML", ctx.attrInnerHTML, i0.ɵɵsanitizeHtml)("title", ctx.attrSafeTitle);
84658465
}
84668466
}
84678467
`;
@@ -8517,14 +8517,14 @@ runInEachFileSystem((os: string) => {
85178517
hostVars: 5,
85188518
hostBindings: function UnsafePropsDirective_HostBindings(rf, ctx) {
85198519
if (rf & 2) {
8520-
i0.ɵɵdomProperty("href", ctx.propHref, i0.ɵɵsanitizeUrlOrResourceUrl)("src", ctx.propSrc, i0.ɵɵsanitizeUrlOrResourceUrl)("action", ctx.propAction, i0.ɵɵsanitizeUrl)("innerHTML", ctx.propInnerHTML, i0.ɵɵsanitizeHtml)("title", ctx.propSafeTitle);
8520+
i0.ɵɵdomProperty("href", ctx.propHref, i0.ɵɵsanitizeUrlOrResourceUrl)("src", ctx.propSrc, i0.ɵɵsanitizeUrlOrResourceUrl)("action", ctx.propAction, i0.ɵɵsanitizeUrlOrResourceUrl)("innerHTML", ctx.propInnerHTML, i0.ɵɵsanitizeHtml)("title", ctx.propSafeTitle);
85218521
}
85228522
}
85238523
`;
85248524
expect(trim(jsContents)).toContain(trim(hostBindingsFn));
85258525
});
85268526

8527-
it('should not generate sanitizers for URL properties in hostBindings fn in Component', () => {
8527+
it('should generate concrete-host URL sanitizers in hostBindings fn in Component', () => {
85288528
env.write(
85298529
`test.ts`,
85308530
`
@@ -8552,8 +8552,40 @@ runInEachFileSystem((os: string) => {
85528552
hostVars: 6,
85538553
hostBindings: function FooCmp_HostBindings(rf, ctx) {
85548554
if (rf & 2) {
8555-
i0.ɵɵdomProperty("src", ctx.srcProp)("href", ctx.hrefProp)("title", ctx.titleProp);
8556-
i0.ɵɵattribute("src", ctx.srcAttr)("href", ctx.hrefAttr)("title", ctx.titleAttr);
8555+
i0.ɵɵdomProperty("src", ctx.srcProp, i0.ɵɵsanitizeUrlOrResourceUrl)("href", ctx.hrefProp, i0.ɵɵsanitizeUrlOrResourceUrl)("title", ctx.titleProp);
8556+
i0.ɵɵattribute("src", ctx.srcAttr, i0.ɵɵsanitizeUrlOrResourceUrl)("href", ctx.hrefAttr, i0.ɵɵsanitizeUrlOrResourceUrl)("title", ctx.titleAttr);
8557+
}
8558+
}
8559+
`;
8560+
expect(trim(jsContents)).toContain(trim(hostBindingsFn));
8561+
});
8562+
8563+
it('should generate sanitizers for pure :not selector host bindings', () => {
8564+
env.write(
8565+
`test.ts`,
8566+
`
8567+
import {Component} from '@angular/core';
8568+
8569+
@Component({
8570+
selector: ':not(iframe)',
8571+
template: '',
8572+
host: {
8573+
'[attr.srcdoc]': 'srcdoc',
8574+
}
8575+
})
8576+
class FooCmp {
8577+
srcdoc: any;
8578+
}
8579+
`,
8580+
);
8581+
8582+
env.driveMain();
8583+
const jsContents = env.getContents('test.js');
8584+
const hostBindingsFn = `
8585+
hostVars: 1,
8586+
hostBindings: function FooCmp_HostBindings(rf, ctx) {
8587+
if (rf & 2) {
8588+
i0.ɵɵattribute("srcdoc", ctx.srcdoc, i0.ɵɵsanitizeHtml);
85578589
}
85588590
}
85598591
`;

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -444,10 +444,12 @@ export class DomElementSchemaRegistry extends ElementSchemaRegistry {
444444

445445
const normalizedTag = normalizeTagName(tagName);
446446
propName = propName.toLowerCase();
447+
const [namespace] = splitNsName(normalizedTag, false);
447448

448449
const securitySchema = SECURITY_SCHEMA();
449450
const ctx =
450451
securitySchema[normalizedTag + '|' + propName] ??
452+
(namespace ? securitySchema[`:${namespace}:*|${propName}`] : undefined) ??
451453
securitySchema['*|' + propName] ??
452454
SecurityContext.NONE;
453455

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

Lines changed: 29 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -39,8 +39,8 @@ export enum SecurityContext {
3939
* Map from tagName|propertyName to SecurityContext. Properties applying to all tags use '*'.
4040
*/
4141
let _SECURITY_SCHEMA!: {[k: string]: SecurityContext};
42-
const SVG_NAMESPACE = 'svg';
43-
const MATH_ML_NAMESPACE = 'math';
42+
export const SVG_NAMESPACE = 'svg';
43+
export const MATH_ML_NAMESPACE = 'math';
4444

4545
/**
4646
* @remarks Keep is a copy of DOM Security Schema.
@@ -71,6 +71,7 @@ export function SECURITY_SCHEMA(): {[k: string]: SecurityContext} {
7171
registerContext(SecurityContext.URL, MATH_ML_NAMESPACE, [
7272
// MathML namespace
7373
// https://crsrc.org/c/third_party/blink/renderer/core/sanitizer/sanitizer.cc;l=753-768;drc=b3eb16372dcd3317d65e9e0265015e322494edcd;bpv=1;bpt=1
74+
['*', ['href', 'xlink:href']],
7475
['annotation', ['href', 'xlink:href']],
7576
['annotation-xml', ['href', 'xlink:href']],
7677
['maction', ['href', 'xlink:href']],
@@ -157,12 +158,36 @@ function registerContext(
157158
specs: readonly [tagName: string, attributeNames: readonly string[]][],
158159
): void {
159160
for (const [element, attributeNames] of specs) {
160-
let tagName =
161-
namespace && element !== '*' && element !== 'unknown' ? `:${namespace}:${element}` : element;
161+
let tagName = namespace && element !== 'unknown' ? `:${namespace}:${element}` : element;
162162
tagName = tagName.toLowerCase();
163163

164164
for (const attr of attributeNames) {
165165
_SECURITY_SCHEMA[`${tagName}|${attr.toLowerCase()}`] = ctx;
166166
}
167167
}
168168
}
169+
170+
export function checkSecurityContext(
171+
tagName: string,
172+
propName: string,
173+
namespace?: string | null,
174+
): SecurityContext {
175+
const schema = SECURITY_SCHEMA();
176+
const normalizedTagName = tagName.toLowerCase();
177+
const normalizedPropName = propName.toLowerCase();
178+
const namespacedContext =
179+
namespace && normalizedTagName !== '*' && normalizedTagName !== 'unknown'
180+
? schema[`:${namespace}:${normalizedTagName}|${normalizedPropName}`]
181+
: undefined;
182+
const namespacedWildcardContext = namespace
183+
? schema[`:${namespace}:*|${normalizedPropName}`]
184+
: undefined;
185+
186+
return (
187+
namespacedContext ??
188+
namespacedWildcardContext ??
189+
schema[`${normalizedTagName}|${normalizedPropName}`] ??
190+
schema[`*|${normalizedPropName}`] ??
191+
SecurityContext.NONE
192+
);
193+
}

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

Lines changed: 49 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ import * as t from '../../../render3/r3_ast';
1717
import {DeferBlockDepsEmitMode, R3ComponentDeferMetadata} from '../../../render3/view/api';
1818
import {icuFromI18nMessage} from '../../../render3/view/i18n/util';
1919
import {DomElementSchemaRegistry} from '../../../schema/dom_element_schema_registry';
20-
import {BindingParser} from '../../../template_parser/binding_parser';
20+
import {BindingParser, calcPossibleSecurityContexts} from '../../../template_parser/binding_parser';
2121
import * as ir from '../ir';
2222

2323
import {
@@ -119,19 +119,21 @@ export function ingestHostBinding(
119119
if (property.isAnimation) {
120120
bindingKind = ir.BindingKind.Animation;
121121
}
122-
const securityContexts = bindingParser
123-
.calcPossibleSecurityContexts(
124-
input.componentSelector,
125-
property.name,
126-
bindingKind === ir.BindingKind.Attribute,
127-
)
128-
.filter((context) => context !== SecurityContext.NONE);
122+
const securityContexts = calcHostBindingSecurityContexts(
123+
bindingParser,
124+
input.componentSelector,
125+
property.name,
126+
bindingKind === ir.BindingKind.Attribute,
127+
);
129128
ingestDomProperty(job, property, bindingKind, securityContexts);
130129
}
131130
for (const [name, expr] of Object.entries(input.attributes) ?? []) {
132-
const securityContexts = bindingParser
133-
.calcPossibleSecurityContexts(input.componentSelector, name, true)
134-
.filter((context) => context !== SecurityContext.NONE);
131+
const securityContexts = calcHostBindingSecurityContexts(
132+
bindingParser,
133+
input.componentSelector,
134+
name,
135+
true,
136+
);
135137
ingestHostAttribute(job, name, expr, securityContexts);
136138
}
137139
for (const event of input.events ?? []) {
@@ -140,6 +142,42 @@ export function ingestHostBinding(
140142
return job;
141143
}
142144

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

0 commit comments

Comments
 (0)