Skip to content

Commit d4111ee

Browse files
alan-agius4alan-agius4
authored
refactor(compiler): remove unnecessary sanitization for safe attributes
Remove sanitization for attributes that cannot execute code (e.g. `javascript: URIs`).
1 parent 193aa33 commit d4111ee

File tree

12 files changed

+34
-61
lines changed

12 files changed

+34
-61
lines changed

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -905,18 +905,19 @@ i0.ɵɵngDeclareClassMetadata({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDE
905905
export class HostBindingImageDir {
906906
constructor() {
907907
this.evil = 'evil';
908+
this.nonEvil = 'nonEvil';
908909
}
909910
}
910911
HostBindingImageDir.ɵfac = i0.ɵɵngDeclareFactory({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: HostBindingImageDir, deps: [], target: i0.ɵɵFactoryTarget.Directive });
911-
HostBindingImageDir.ɵdir = i0.ɵɵngDeclareDirective({ minVersion: "14.0.0", version: "0.0.0-PLACEHOLDER", type: HostBindingImageDir, isStandalone: true, selector: "img[hostBindingImgDir]", host: { properties: { "innerHtml": "evil", "attr.style": "evil", "src": "evil" } }, ngImport: i0 });
912+
HostBindingImageDir.ɵdir = i0.ɵɵngDeclareDirective({ minVersion: "14.0.0", version: "0.0.0-PLACEHOLDER", type: HostBindingImageDir, isStandalone: true, selector: "img[hostBindingImgDir]", host: { properties: { "innerHtml": "evil", "attr.style": "evil", "src": "nonEvil" } }, ngImport: i0 });
912913
i0.ɵɵngDeclareClassMetadata({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: HostBindingImageDir, decorators: [{
913914
type: Directive,
914915
args: [{
915916
selector: 'img[hostBindingImgDir]',
916917
host: {
917918
'[innerHtml]': 'evil',
918919
'[attr.style]': 'evil',
919-
'[src]': 'evil',
920+
'[src]': 'nonEvil',
920921
},
921922
}]
922923
}] });
@@ -969,6 +970,7 @@ export declare class HostBindingLinkDir {
969970
}
970971
export declare class HostBindingImageDir {
971972
evil: string;
973+
nonEvil: string;
972974
static ɵfac: i0.ɵɵFactoryDeclaration<HostBindingImageDir, never>;
973975
static ɵdir: i0.ɵɵDirectiveDeclaration<HostBindingImageDir, "img[hostBindingImgDir]", never, {}, {}, never, never, true, never>;
974976
}

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,14 +7,14 @@ hostBindings: function HostBindingLinkDir_HostBindings(rf, ctx) {
77
…
88
hostBindings: function HostBindingImageDir_HostBindings(rf, ctx) {
99
if (rf & 2) {
10-
$r3$.ɵɵdomProperty("innerHTML", ctx.evil, $r3$.ɵɵsanitizeHtml)("src", ctx.evil, $r3$.ɵɵsanitizeUrl);
11-
$r3$.ɵɵattribute("style", ctx.evil, $r3$.ɵɵsanitizeStyle);
10+
i0.ɵɵdomProperty("innerHTML", ctx.evil, i0.ɵɵsanitizeHtml)("src", ctx.nonEvil, i0.ɵɵsanitizeUrl);
11+
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, $r3$.ɵɵsanitizeResourceUrl)("sandbox", ctx.evil, $r3$.ɵɵvalidateAttribute);
17+
$r3$.ɵɵdomProperty("innerHTML", ctx.evil, $r3$.ɵɵsanitizeHtml)("src", ctx.evil, i0.ɵɵsanitizeResourceUrl)("sandbox", ctx.evil, $r3$.ɵɵvalidateAttribute);
1818
$r3$.ɵɵattribute("style", ctx.evil, $r3$.ɵɵsanitizeStyle)("attributeName", ctx.nonEvil);
1919
}
2020
}

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,11 +17,12 @@ export class HostBindingLinkDir {
1717
host: {
1818
'[innerHtml]': 'evil',
1919
'[attr.style]': 'evil',
20-
'[src]': 'evil',
20+
'[src]': 'nonEvil',
2121
},
2222
})
2323
export class HostBindingImageDir {
2424
evil = 'evil';
25+
nonEvil = 'nonEvil';
2526
}
2627

2728
@Directive({

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -827,14 +827,15 @@ import * as i0 from "@angular/core";
827827
export class MyComponent {
828828
constructor() {
829829
this.evil = 'evil';
830+
this.nonEvil = 'nonEvil';
830831
}
831832
}
832833
MyComponent.ɵfac = i0.ɵɵngDeclareFactory({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyComponent, deps: [], target: i0.ɵɵFactoryTarget.Component });
833834
MyComponent.ɵcmp = i0.ɵɵngDeclareComponent({ minVersion: "14.0.0", version: "0.0.0-PLACEHOLDER", type: MyComponent, isStandalone: true, selector: "my-component", ngImport: i0, template: `
834835
<div [innerHtml]="evil"></div>
835836
<link [href]="evil" />
836837
<div [attr.style]="evil"></div>
837-
<img [src]="evil" />
838+
<img [src]="nonEvil" />
838839
<iframe [sandbox]="evil"></iframe>
839840
<a href="{{evil}}{{evil}}"></a>
840841
<div attr.style="{{evil}}{{evil}}"></div>
@@ -847,7 +848,7 @@ i0.ɵɵngDeclareClassMetadata({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDE
847848
<div [innerHtml]="evil"></div>
848849
<link [href]="evil" />
849850
<div [attr.style]="evil"></div>
850-
<img [src]="evil" />
851+
<img [src]="nonEvil" />
851852
<iframe [sandbox]="evil"></iframe>
852853
<a href="{{evil}}{{evil}}"></a>
853854
<div attr.style="{{evil}}{{evil}}"></div>
@@ -861,6 +862,7 @@ i0.ɵɵngDeclareClassMetadata({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDE
861862
import * as i0 from "@angular/core";
862863
export declare class MyComponent {
863864
evil: string;
865+
nonEvil: string;
864866
static ɵfac: i0.ɵɵFactoryDeclaration<MyComponent, never>;
865867
static ɵcmp: i0.ɵɵComponentDeclaration<MyComponent, "my-component", never, {}, {}, never, never, true, never>;
866868
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ template: function MyComponent_Template(rf, ctx) {
99
$r3$.ɵɵadvance();
1010
$r3$.ɵɵattribute("style", ctx.evil, $r3$.ɵɵsanitizeStyle);
1111
$r3$.ɵɵadvance();
12-
$r3$.ɵɵdomProperty("src", ctx.evil, $r3$.ɵɵsanitizeUrl);
12+
$r3$.ɵɵdomProperty("src", ctx.nonEvil, $r3$.ɵɵsanitizeUrl);
1313
$r3$.ɵɵadvance();
1414
$r3$.ɵɵdomProperty("sandbox", ctx.evil, $r3$.ɵɵvalidateAttribute);
1515
$r3$.ɵɵadvance();

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,13 @@ import {Component} from '@angular/core';
66
<div [innerHtml]="evil"></div>
77
<link [href]="evil" />
88
<div [attr.style]="evil"></div>
9-
<img [src]="evil" />
9+
<img [src]="nonEvil" />
1010
<iframe [sandbox]="evil"></iframe>
1111
<a href="{{evil}}{{evil}}"></a>
1212
<div attr.style="{{evil}}{{evil}}"></div>
1313
`
1414
})
1515
export class MyComponent {
1616
evil = 'evil';
17+
nonEvil = 'nonEvil';
1718
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8497,7 +8497,7 @@ runInEachFileSystem((os: string) => {
84978497
hostVars: 6,
84988498
hostBindings: function UnsafeAttrsDirective_HostBindings(rf, ctx) {
84998499
if (rf & 2) {
8500-
i0.ɵɵattribute("href", ctx.attrHref, i0.ɵɵsanitizeUrlOrResourceUrl)("src", ctx.attrSrc, i0.ɵɵsanitizeUrlOrResourceUrl)("action", ctx.attrAction, i0.ɵɵsanitizeUrl)("profile", ctx.attrProfile, i0.ɵɵsanitizeResourceUrl)("innerHTML", ctx.attrInnerHTML, i0.ɵɵsanitizeHtml)("title", ctx.attrSafeTitle);
8500+
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);
85018501
}
85028502
}
85038503
`;

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

Lines changed: 4 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -32,23 +32,9 @@ export function SECURITY_SCHEMA(): {[k: string]: SecurityContext} {
3232
registerContext(SecurityContext.URL, [
3333
'*|formAction',
3434
'area|href',
35-
'area|ping',
36-
'audio|src',
3735
'a|href',
3836
'a|xlink:href',
39-
'a|ping',
40-
'blockquote|cite',
41-
'body|background',
42-
'del|cite',
4337
'form|action',
44-
'img|src',
45-
'input|src',
46-
'ins|cite',
47-
'q|cite',
48-
'source|src',
49-
'track|src',
50-
'video|poster',
51-
'video|src',
5238

5339
// MathML namespace
5440
// https://crsrc.org/c/third_party/blink/renderer/core/sanitizer/sanitizer.cc;l=753-768;drc=b3eb16372dcd3317d65e9e0265015e322494edcd;bpv=1;bpt=1
@@ -118,19 +104,18 @@ export function SECURITY_SCHEMA(): {[k: string]: SecurityContext} {
118104
'semantics|xlink:href',
119105
'none|href',
120106
'none|xlink:href',
107+
108+
// The below two items are safe and should be removed but they require a G3 clean-up as a small number of tests fail.
109+
'img|src',
110+
'video|src',
121111
]);
122112

123113
registerContext(SecurityContext.RESOURCE_URL, [
124-
'applet|code',
125-
'applet|codebase',
126114
'base|href',
127115
'embed|src',
128116
'frame|src',
129-
'head|profile',
130-
'html|manifest',
131117
'iframe|src',
132118
'link|href',
133-
'media|src',
134119
'object|codebase',
135120
'object|data',
136121
'script|src',

‎packages/compiler/test/schema/dom_element_schema_registry_spec.ts‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -156,7 +156,6 @@ If 'onAnything' is a directive input, make sure the directive is imported by the
156156
expect(registry.securityContext('p', 'innerHTML', false)).toBe(SecurityContext.HTML);
157157
expect(registry.securityContext('a', 'href', false)).toBe(SecurityContext.URL);
158158
expect(registry.securityContext('a', 'style', false)).toBe(SecurityContext.STYLE);
159-
expect(registry.securityContext('ins', 'cite', false)).toBe(SecurityContext.URL);
160159
expect(registry.securityContext('base', 'href', false)).toBe(SecurityContext.RESOURCE_URL);
161160
});
162161

‎packages/core/test/acceptance/host_binding_spec.ts‎

Lines changed: 0 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1607,23 +1607,6 @@ describe('host bindings', () => {
16071607
true,
16081608
true,
16091609
);
1610-
verify(
1611-
'blockquote',
1612-
'cite',
1613-
'javascript:alert(2)',
1614-
'unsafe:javascript:alert(2)',
1615-
bypassSanitizationTrustUrl,
1616-
);
1617-
verify('blockquote', 'cite', 'javascript:alert(2.1)', 'unsafe:javascript:alert(2.1)', identity);
1618-
verify(
1619-
'blockquote',
1620-
'cite',
1621-
'javascript:alert(2.2)',
1622-
'unsafe:javascript:alert(2.2)',
1623-
bypassSanitizationTrustHtml,
1624-
true,
1625-
true,
1626-
);
16271610
verify(
16281611
'b',
16291612
'innerHTML',

0 commit comments

Comments
 (0)