Skip to content

Commit 7c42e2e

Browse files
alan-agius4crisbeto
alan-agius4
authored and
crisbeto
committed
fix(compiler): prevent XSS via SVG animation attributeName and MathML/SVG URLs
This commit implements a security fix to prevent XSS vulnerabilities where SVG animation elements (`<animate>`, `<set>`, etc.) could be used to modify the `href` or `xlink:href` attributes of other elements to `javascript:` URLs. The fix introduces a runtime validation step: - A new [ɵɵValidateAttribute](cci:1://file:///usr/local/google/home/alanagius/git/angular/packages/core/src/sanitization/sanitization.ts:276:0-288:1) instruction is used when `attributeName` is bound on SVG animation elements. - If executed, a `RuntimeError` is thrown, preventing the binding. - The compiler now identifies `attributeName` on SVG animation elements as security-sensitive and injects this validation. Additionally, the DOM security schema has been updated to include a comprehensive list of MathML and SVG elements that accept `href` or `xlink:href` attributes, ensuring they are correctly treated as `SecurityContext.URL` and sanitized. This prevents malicious URLs from being bound to these attributes. http://b/463880509
1 parent 92db2ba commit 7c42e2e

File tree

18 files changed

+327
-154
lines changed

18 files changed

+327
-154
lines changed

‎goldens/public-api/core/errors.api.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -162,6 +162,8 @@ export const enum RuntimeErrorCode {
162162
// (undocumented)
163163
UNKNOWN_ELEMENT = 304,
164164
// (undocumented)
165+
UNSAFE_ATTRIBUTE_BINDING = -910,
166+
// @deprecated (undocumented)
165167
UNSAFE_IFRAME_ATTRS = -910,
166168
// (undocumented)
167169
UNSAFE_VALUE_IN_RESOURCE_URL = 904,

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,6 @@ template: function MyComponent_Template(rf, ctx) {
33
if (rf & 1) {
44
i0.ɵɵelement(0, "iframe", 0);
55
} if (rf & 2) {
6-
i0.ɵɵattribute("fetchpriority", "low", i0.ɵɵvalidateIframeAttribute)("allowfullscreen", ctx.fullscreen, i0.ɵɵvalidateIframeAttribute);
6+
i0.ɵɵattribute("fetchpriority", "low", i0.ɵɵvalidateAttribute)("allowfullscreen", ctx.fullscreen, i0.ɵɵvalidateAttribute);
77
}
88
}

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

Lines changed: 27 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -843,7 +843,7 @@ export class HostBindingDir {
843843
}
844844
}
845845
HostBindingDir.ɵfac = i0.ɵɵngDeclareFactory({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: HostBindingDir, deps: [], target: i0.ɵɵFactoryTarget.Directive });
846-
HostBindingDir.ɵdir = i0.ɵɵngDeclareDirective({ minVersion: "14.0.0", version: "0.0.0-PLACEHOLDER", type: HostBindingDir, isStandalone: true, selector: "[hostBindingDir]", host: { properties: { "innerHtml": "evil", "href": "evil", "attr.style": "evil", "src": "evil", "sandbox": "evil" } }, ngImport: i0 });
846+
HostBindingDir.ɵdir = i0.ɵɵngDeclareDirective({ minVersion: "14.0.0", version: "0.0.0-PLACEHOLDER", type: HostBindingDir, isStandalone: true, selector: "[hostBindingDir]", host: { properties: { "innerHtml": "evil", "href": "evil", "attr.style": "evil", "src": "evil", "sandbox": "evil", "attr.attributeName": "nonEvil" } }, ngImport: i0 });
847847
i0.ɵɵngDeclareClassMetadata({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: HostBindingDir, decorators: [{
848848
type: Directive,
849849
args: [{
@@ -854,16 +854,18 @@ i0.ɵɵngDeclareClassMetadata({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDE
854854
'[attr.style]': 'evil',
855855
'[src]': 'evil',
856856
'[sandbox]': 'evil',
857+
'[attr.attributeName]': 'nonEvil',
857858
},
858859
}]
859860
}] });
860861
export class HostBindingDir2 {
861862
constructor() {
862863
this.evil = 'evil';
864+
this.nonEvil = 'nonEvil';
863865
}
864866
}
865867
HostBindingDir2.ɵfac = i0.ɵɵngDeclareFactory({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: HostBindingDir2, deps: [], target: i0.ɵɵFactoryTarget.Directive });
866-
HostBindingDir2.ɵdir = i0.ɵɵngDeclareDirective({ minVersion: "14.0.0", version: "0.0.0-PLACEHOLDER", type: HostBindingDir2, isStandalone: true, selector: "a", host: { properties: { "innerHtml": "evil", "href": "evil", "attr.style": "evil", "src": "evil", "sandbox": "evil" } }, ngImport: i0 });
868+
HostBindingDir2.ɵdir = i0.ɵɵngDeclareDirective({ minVersion: "14.0.0", version: "0.0.0-PLACEHOLDER", type: HostBindingDir2, isStandalone: true, selector: "a", host: { properties: { "innerHtml": "evil", "href": "evil", "attr.style": "evil", "src": "evil", "sandbox": "nonEvil" } }, ngImport: i0 });
867869
i0.ɵɵngDeclareClassMetadata({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: HostBindingDir2, decorators: [{
868870
type: Directive,
869871
args: [{
@@ -873,7 +875,23 @@ i0.ɵɵngDeclareClassMetadata({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDE
873875
'[href]': 'evil',
874876
'[attr.style]': 'evil',
875877
'[src]': 'evil',
876-
'[sandbox]': 'evil',
878+
'[sandbox]': 'nonEvil',
879+
},
880+
}]
881+
}] });
882+
export class HostBindingSvgAnimateDir {
883+
constructor() {
884+
this.evil = 'evil';
885+
}
886+
}
887+
HostBindingSvgAnimateDir.ɵfac = i0.ɵɵngDeclareFactory({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: HostBindingSvgAnimateDir, deps: [], target: i0.ɵɵFactoryTarget.Directive });
888+
HostBindingSvgAnimateDir.ɵdir = i0.ɵɵngDeclareDirective({ minVersion: "14.0.0", version: "0.0.0-PLACEHOLDER", type: HostBindingSvgAnimateDir, isStandalone: true, selector: "animateMotion[hostBindingSvgAnimateDir]", host: { properties: { "attr.attributeName": "evil" } }, ngImport: i0 });
889+
i0.ɵɵngDeclareClassMetadata({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: HostBindingSvgAnimateDir, decorators: [{
890+
type: Directive,
891+
args: [{
892+
selector: 'animateMotion[hostBindingSvgAnimateDir]',
893+
host: {
894+
'[attr.attributeName]': 'evil',
877895
},
878896
}]
879897
}] });
@@ -889,9 +907,15 @@ export declare class HostBindingDir {
889907
}
890908
export declare class HostBindingDir2 {
891909
evil: string;
910+
nonEvil: string;
892911
static ɵfac: i0.ɵɵFactoryDeclaration<HostBindingDir2, never>;
893912
static ɵdir: i0.ɵɵDirectiveDeclaration<HostBindingDir2, "a", never, {}, {}, never, never, true, never>;
894913
}
914+
export declare class HostBindingSvgAnimateDir {
915+
evil: string;
916+
static ɵfac: i0.ɵɵFactoryDeclaration<HostBindingSvgAnimateDir, never>;
917+
static ɵdir: i0.ɵɵDirectiveDeclaration<HostBindingSvgAnimateDir, "animateMotion[hostBindingSvgAnimateDir]", never, {}, {}, never, never, true, never>;
918+
}
895919

896920
/****************************************************************************************************
897921
* PARTIAL FILE: security_sensitive_constant_attributes.js

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

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,19 @@
11
hostBindings: function HostBindingDir_HostBindings(rf, ctx) {
22
if (rf & 2) {
3-
i0.ɵɵhostProperty("innerHtml", ctx.evil, i0.ɵɵsanitizeHtml)("href", ctx.evil, i0.ɵɵsanitizeUrlOrResourceUrl)("src", ctx.evil, i0.ɵɵsanitizeUrlOrResourceUrl)("sandbox", ctx.evil, i0.ɵɵvalidateIframeAttribute);
4-
i0.ɵɵattribute("style", ctx.evil, i0.ɵɵsanitizeStyle);
3+
i0.ɵɵhostProperty("innerHtml", ctx.evil, i0.ɵɵsanitizeHtml)("href", ctx.evil, i0.ɵɵsanitizeUrlOrResourceUrl)("src", ctx.evil, i0.ɵɵsanitizeUrlOrResourceUrl)("sandbox", ctx.evil, i0.ɵɵvalidateAttribute);
4+
i0.ɵɵattribute("style", ctx.evil, i0.ɵɵsanitizeStyle)("attributeName", ctx.nonEvil, i0.ɵɵvalidateAttribute);
55
}
66
}
77
…
88
hostBindings: function HostBindingDir2_HostBindings(rf, ctx) {
99
if (rf & 2) {
10-
i0.ɵɵhostProperty("innerHtml", ctx.evil, i0.ɵɵsanitizeHtml)("href", ctx.evil, i0.ɵɵsanitizeUrl)("src", ctx.evil)("sandbox", ctx.evil, i0.ɵɵvalidateIframeAttribute);
10+
i0.ɵɵhostProperty("innerHtml", ctx.evil, i0.ɵɵsanitizeHtml)("href", ctx.evil, i0.ɵɵsanitizeUrl)("src", ctx.evil)("sandbox", ctx.nonEvil);
1111
i0.ɵɵattribute("style", ctx.evil, i0.ɵɵsanitizeStyle);
1212
}
1313
}
14+
…
15+
hostBindings: function HostBindingSvgAnimateDir_HostBindings(rf, ctx) {
16+
if (rf & 2) {
17+
i0.ɵɵattribute("attributeName", ctx.evil, i0.ɵɵvalidateAttribute);
18+
}
19+
}

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

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import {Directive} from '@angular/core';
88
'[attr.style]': 'evil',
99
'[src]': 'evil',
1010
'[sandbox]': 'evil',
11+
'[attr.attributeName]': 'nonEvil',
1112
},
1213
})
1314
export class HostBindingDir {
@@ -21,9 +22,20 @@ export class HostBindingDir {
2122
'[href]': 'evil',
2223
'[attr.style]': 'evil',
2324
'[src]': 'evil',
24-
'[sandbox]': 'evil',
25+
'[sandbox]': 'nonEvil',
2526
},
2627
})
2728
export class HostBindingDir2 {
2829
evil = 'evil';
30+
nonEvil = 'nonEvil';
31+
}
32+
33+
@Directive({
34+
selector: 'animateMotion[hostBindingSvgAnimateDir]',
35+
host: {
36+
'[attr.attributeName]': 'evil',
37+
},
38+
})
39+
export class HostBindingSvgAnimateDir {
40+
evil = 'evil';
2941
}

‎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
@@ -11,7 +11,7 @@ template: function MyComponent_Template(rf, ctx) {
1111
i0.ɵɵadvance();
1212
i0.ɵɵproperty("src", ctx.evil, i0.ɵɵsanitizeUrl);
1313
i0.ɵɵadvance();
14-
i0.ɵɵproperty("sandbox", ctx.evil, i0.ɵɵvalidateIframeAttribute);
14+
i0.ɵɵproperty("sandbox", ctx.evil, i0.ɵɵvalidateAttribute);
1515
i0.ɵɵadvance();
1616
i0.ɵɵpropertyInterpolate2("href", "", ctx.evil, "", ctx.evil, "", i0.ɵɵsanitizeUrl);
1717
i0.ɵɵadvance();

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

Lines changed: 33 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9009,6 +9009,33 @@ runInEachFileSystem((os: string) => {
90099009
});
90109010
});
90119011

9012+
describe('SVG animation processing', () => {
9013+
it('should generate SVG animation validation instruction', () => {
9014+
env.write(
9015+
'test.ts',
9016+
`
9017+
import {Component} from '@angular/core';
9018+
9019+
@Component({
9020+
selector: 'test-cmp',
9021+
template: '<svg><animate [attr.attributeName]="attr"></animate></svg>',
9022+
standalone: false,
9023+
})
9024+
export class TestCmp {
9025+
attr = 'opacity';
9026+
}
9027+
`,
9028+
);
9029+
9030+
env.driveMain();
9031+
9032+
const jsContents = env.getContents('test.js');
9033+
expect(jsContents).toContain(
9034+
'i0.ɵɵattribute("attributeName", ctx.attr, i0.ɵɵvalidateAttribute);',
9035+
);
9036+
});
9037+
});
9038+
90129039
describe('inline resources', () => {
90139040
it('should process inline <style> tags', () => {
90149041
env.write(
@@ -9430,11 +9457,11 @@ runInEachFileSystem((os: string) => {
94309457
// Only `sandbox` has an extra validation fn (since it's security-sensitive),
94319458
// the `title` property doesn't have an extra validation fn.
94329459
expect(jsContents).toContain(
9433-
'ɵɵproperty("sandbox", "", i0.ɵɵvalidateIframeAttribute)("title", "Hi!")',
9460+
'ɵɵproperty("sandbox", "", i0.ɵɵvalidateAttribute)("title", "Hi!")',
94349461
);
94359462

94369463
// The `allow` property is also security-sensitive, thus an extra validation fn.
9437-
expect(jsContents).toContain('ɵɵattribute("allow", "", i0.ɵɵvalidateIframeAttribute)');
9464+
expect(jsContents).toContain('ɵɵattribute("allow", "", i0.ɵɵvalidateAttribute)');
94389465
});
94399466

94409467
it(
@@ -9464,7 +9491,7 @@ runInEachFileSystem((os: string) => {
94649491
// Make sure that the `sandbox` has an extra validation fn,
94659492
// and the check is case-insensitive (since the `setAttribute` DOM API
94669493
// is case-insensitive as well).
9467-
expect(jsContents).toContain('ɵɵattribute("SANDBOX", "", i0.ɵɵvalidateIframeAttribute)');
9494+
expect(jsContents).toContain('ɵɵattribute("SANDBOX", "", i0.ɵɵvalidateAttribute)');
94689495
},
94699496
);
94709497

@@ -9523,11 +9550,11 @@ runInEachFileSystem((os: string) => {
95239550
// The `sandbox` is potentially a security-sensitive attribute of an <iframe>.
95249551
// Generate an extra validation function to invoke at runtime, which would
95259552
// check if an underlying host element is an <iframe>.
9526-
expect(jsContents).toContain('ɵɵhostProperty("sandbox", "", i0.ɵɵvalidateIframeAttribute)');
9553+
expect(jsContents).toContain('ɵɵhostProperty("sandbox", "", i0.ɵɵvalidateAttribute)');
95279554

95289555
// Similar to the above, but for an attribute binding (host attributes are
95299556
// represented via `ɵɵattribute`).
9530-
expect(jsContents).toContain('ɵɵattribute("allow", "", i0.ɵɵvalidateIframeAttribute)');
9557+
expect(jsContents).toContain('ɵɵattribute("allow", "", i0.ɵɵvalidateAttribute)');
95319558
});
95329559

95339560
it(
@@ -9553,7 +9580,7 @@ runInEachFileSystem((os: string) => {
95539580

95549581
// Make sure that we generate a validation fn for the `sandbox` attribute,
95559582
// even when it was declared as `SANDBOX`.
9556-
expect(jsContents).toContain('ɵɵattribute("SANDBOX", "", i0.ɵɵvalidateIframeAttribute)');
9583+
expect(jsContents).toContain('ɵɵattribute("SANDBOX", "", i0.ɵɵvalidateAttribute)');
95579584
},
95589585
);
95599586
});

‎packages/compiler/src/core.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,7 @@ export enum SecurityContext {
8383
SCRIPT = 3,
8484
URL = 4,
8585
RESOURCE_URL = 5,
86+
ATTRIBUTE_NO_BINDING = 6,
8687
}
8788

8889
/**

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -570,6 +570,10 @@ export class Identifiers {
570570
// sanitization-related functions
571571
static sanitizeHtml: o.ExternalReference = {name: 'ɵɵsanitizeHtml', moduleName: CORE};
572572
static sanitizeStyle: o.ExternalReference = {name: 'ɵɵsanitizeStyle', moduleName: CORE};
573+
static validateAttribute: o.ExternalReference = {
574+
name: 'ɵɵvalidateAttribute',
575+
moduleName: CORE,
576+
};
573577
static sanitizeResourceUrl: o.ExternalReference = {
574578
name: 'ɵɵsanitizeResourceUrl',
575579
moduleName: CORE,
@@ -585,10 +589,6 @@ export class Identifiers {
585589
name: 'ɵɵtrustConstantResourceUrl',
586590
moduleName: CORE,
587591
};
588-
static validateIframeAttribute: o.ExternalReference = {
589-
name: 'ɵɵvalidateIframeAttribute',
590-
moduleName: CORE,
591-
};
592592

593593
// type-checking
594594
static InputSignalBrandWriteType = {name: 'ɵINPUT_SIGNAL_BRAND_WRITE_TYPE', moduleName: CORE};

0 commit comments

Comments
 (0)