Skip to content

Commit d1ca8ae

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 136e923 commit d1ca8ae

File tree

18 files changed

+325
-152
lines changed

18 files changed

+325
-152
lines changed

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -182,6 +182,8 @@ export const enum RuntimeErrorCode {
182182
// (undocumented)
183183
UNKNOWN_ELEMENT = 304,
184184
// (undocumented)
185+
UNSAFE_ATTRIBUTE_BINDING = -910,
186+
// @deprecated (undocumented)
185187
UNSAFE_IFRAME_ATTRS = -910,
186188
// (undocumented)
187189
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: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -881,7 +881,7 @@ export class HostBindingDir {
881881
}
882882
}
883883
HostBindingDir.ɵfac = i0.ɵɵngDeclareFactory({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: HostBindingDir, deps: [], target: i0.ɵɵFactoryTarget.Directive });
884-
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 });
884+
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 });
885885
i0.ɵɵngDeclareClassMetadata({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: HostBindingDir, decorators: [{
886886
type: Directive,
887887
args: [{
@@ -892,12 +892,14 @@ i0.ɵɵngDeclareClassMetadata({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDE
892892
'[attr.style]': 'evil',
893893
'[src]': 'evil',
894894
'[sandbox]': 'evil',
895+
'[attr.attributeName]': 'nonEvil',
895896
},
896897
}]
897898
}] });
898899
export class HostBindingDir2 {
899900
constructor() {
900901
this.evil = 'evil';
902+
this.nonEvil = 'nonEvil';
901903
}
902904
}
903905
HostBindingDir2.ɵfac = i0.ɵɵngDeclareFactory({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: HostBindingDir2, deps: [], target: i0.ɵɵFactoryTarget.Directive });
@@ -915,6 +917,22 @@ i0.ɵɵngDeclareClassMetadata({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDE
915917
},
916918
}]
917919
}] });
920+
export class HostBindingSvgAnimateDir {
921+
constructor() {
922+
this.evil = 'evil';
923+
}
924+
}
925+
HostBindingSvgAnimateDir.ɵfac = i0.ɵɵngDeclareFactory({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: HostBindingSvgAnimateDir, deps: [], target: i0.ɵɵFactoryTarget.Directive });
926+
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 });
927+
i0.ɵɵngDeclareClassMetadata({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: HostBindingSvgAnimateDir, decorators: [{
928+
type: Directive,
929+
args: [{
930+
selector: 'animateMotion[hostBindingSvgAnimateDir]',
931+
host: {
932+
'[attr.attributeName]': 'evil',
933+
},
934+
}]
935+
}] });
918936

919937
/****************************************************************************************************
920938
* PARTIAL FILE: sanitization.d.ts
@@ -927,9 +945,15 @@ export declare class HostBindingDir {
927945
}
928946
export declare class HostBindingDir2 {
929947
evil: string;
948+
nonEvil: string;
930949
static ɵfac: i0.ɵɵFactoryDeclaration<HostBindingDir2, never>;
931950
static ɵdir: i0.ɵɵDirectiveDeclaration<HostBindingDir2, "a", never, {}, {}, never, never, true, never>;
932951
}
952+
export declare class HostBindingSvgAnimateDir {
953+
evil: string;
954+
static ɵfac: i0.ɵɵFactoryDeclaration<HostBindingSvgAnimateDir, never>;
955+
static ɵdir: i0.ɵɵDirectiveDeclaration<HostBindingSvgAnimateDir, "animateMotion[hostBindingSvgAnimateDir]", never, {}, {}, never, never, true, never>;
956+
}
933957

934958
/****************************************************************************************************
935959
* PARTIAL FILE: security_sensitive_constant_attributes.js

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

Lines changed: 10 additions & 4 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-
$r3$.ɵɵdomProperty("innerHTML", ctx.evil, $r3$.ɵɵsanitizeHtml)("href", ctx.evil, $r3$.ɵɵsanitizeUrlOrResourceUrl)("src", ctx.evil, $r3$.ɵɵsanitizeUrlOrResourceUrl)("sandbox", ctx.evil, $r3$.ɵɵvalidateIframeAttribute);
4-
$r3$.ɵɵattribute("style", ctx.evil, $r3$.ɵɵsanitizeStyle);
3+
i0.ɵɵdomProperty("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-
$r3$.ɵɵdomProperty("innerHTML", ctx.evil, $r3$.ɵɵsanitizeHtml)("href", ctx.evil, $r3$.ɵɵsanitizeUrl)("src", ctx.evil)("sandbox", ctx.evil, $r3$.ɵɵvalidateIframeAttribute);
11-
$r3$.ɵɵattribute("style", ctx.evil, $r3$.ɵɵsanitizeStyle);
10+
i0.ɵɵdomProperty("innerHTML", ctx.evil, i0.ɵɵsanitizeHtml)("href", ctx.evil, i0.ɵɵsanitizeUrl)("src", ctx.evil)("sandbox", ctx.evil);
11+
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: 12 additions & 0 deletions
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 {
@@ -26,4 +27,15 @@ export class HostBindingDir {
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
$r3$.ɵɵadvance();
1212
$r3$.ɵɵdomProperty("src", ctx.evil, $r3$.ɵɵsanitizeUrl);
1313
$r3$.ɵɵadvance();
14-
$r3$.ɵɵdomProperty("sandbox", ctx.evil, $r3$.ɵɵvalidateIframeAttribute);
14+
$r3$.ɵɵdomProperty("sandbox", ctx.evil, $r3$.ɵɵvalidateAttribute);
1515
$r3$.ɵɵadvance();
1616
$r3$.ɵɵdomProperty("href", $r3$.ɵɵinterpolate2("", ctx.evil, "", ctx.evil), $r3$.ɵɵsanitizeUrl);
1717
$r3$.ɵɵadvance();

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

Lines changed: 33 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9261,6 +9261,33 @@ runInEachFileSystem((os: string) => {
92619261
});
92629262
});
92639263

9264+
describe('SVG animation processing', () => {
9265+
it('should generate SVG animation validation instruction', () => {
9266+
env.write(
9267+
'test.ts',
9268+
`
9269+
import {Component} from '@angular/core';
9270+
9271+
@Component({
9272+
selector: 'test-cmp',
9273+
template: '<svg><animate [attr.attributeName]="attr"></animate></svg>',
9274+
standalone: false,
9275+
})
9276+
export class TestCmp {
9277+
attr = 'opacity';
9278+
}
9279+
`,
9280+
);
9281+
9282+
env.driveMain();
9283+
9284+
const jsContents = env.getContents('test.js');
9285+
expect(jsContents).toContain(
9286+
'i0.ɵɵattribute("attributeName", ctx.attr, i0.ɵɵvalidateAttribute);',
9287+
);
9288+
});
9289+
});
9290+
92649291
describe('inline resources', () => {
92659292
it('should process inline <style> tags', () => {
92669293
env.write(
@@ -9682,11 +9709,11 @@ runInEachFileSystem((os: string) => {
96829709
// Only `sandbox` has an extra validation fn (since it's security-sensitive),
96839710
// the `title` property doesn't have an extra validation fn.
96849711
expect(jsContents).toContain(
9685-
'ɵɵdomProperty("sandbox", "", i0.ɵɵvalidateIframeAttribute)("title", "Hi!")',
9712+
'ɵɵdomProperty("sandbox", "", i0.ɵɵvalidateAttribute)("title", "Hi!")',
96869713
);
96879714

96889715
// The `allow` property is also security-sensitive, thus an extra validation fn.
9689-
expect(jsContents).toContain('ɵɵattribute("allow", "", i0.ɵɵvalidateIframeAttribute)');
9716+
expect(jsContents).toContain('ɵɵattribute("allow", "", i0.ɵɵvalidateAttribute)');
96909717
});
96919718

96929719
it(
@@ -9716,7 +9743,7 @@ runInEachFileSystem((os: string) => {
97169743
// Make sure that the `sandbox` has an extra validation fn,
97179744
// and the check is case-insensitive (since the `setAttribute` DOM API
97189745
// is case-insensitive as well).
9719-
expect(jsContents).toContain('ɵɵattribute("SANDBOX", "", i0.ɵɵvalidateIframeAttribute)');
9746+
expect(jsContents).toContain('ɵɵattribute("SANDBOX", "", i0.ɵɵvalidateAttribute)');
97209747
},
97219748
);
97229749

@@ -9775,11 +9802,11 @@ runInEachFileSystem((os: string) => {
97759802
// The `sandbox` is potentially a security-sensitive attribute of an <iframe>.
97769803
// Generate an extra validation function to invoke at runtime, which would
97779804
// check if an underlying host element is an <iframe>.
9778-
expect(jsContents).toContain('ɵɵdomProperty("sandbox", "", i0.ɵɵvalidateIframeAttribute)');
9805+
expect(jsContents).toContain('ɵɵdomProperty("sandbox", "", i0.ɵɵvalidateAttribute)');
97799806

97809807
// Similar to the above, but for an attribute binding (host attributes are
97819808
// represented via `ɵɵattribute`).
9782-
expect(jsContents).toContain('ɵɵattribute("allow", "", i0.ɵɵvalidateIframeAttribute)');
9809+
expect(jsContents).toContain('ɵɵattribute("allow", "", i0.ɵɵvalidateAttribute)');
97839810
});
97849811

97859812
it(
@@ -9805,7 +9832,7 @@ runInEachFileSystem((os: string) => {
98059832

98069833
// Make sure that we generate a validation fn for the `sandbox` attribute,
98079834
// even when it was declared as `SANDBOX`.
9808-
expect(jsContents).toContain('ɵɵattribute("SANDBOX", "", i0.ɵɵvalidateIframeAttribute)');
9835+
expect(jsContents).toContain('ɵɵattribute("SANDBOX", "", i0.ɵɵvalidateAttribute)');
98099836
},
98109837
);
98119838
});

‎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
@@ -457,6 +457,10 @@ export class Identifiers {
457457
// sanitization-related functions
458458
static sanitizeHtml: o.ExternalReference = {name: 'ɵɵsanitizeHtml', moduleName: CORE};
459459
static sanitizeStyle: o.ExternalReference = {name: 'ɵɵsanitizeStyle', moduleName: CORE};
460+
static validateAttribute: o.ExternalReference = {
461+
name: 'ɵɵvalidateAttribute',
462+
moduleName: CORE,
463+
};
460464
static sanitizeResourceUrl: o.ExternalReference = {
461465
name: 'ɵɵsanitizeResourceUrl',
462466
moduleName: CORE,
@@ -472,10 +476,6 @@ export class Identifiers {
472476
name: 'ɵɵtrustConstantResourceUrl',
473477
moduleName: CORE,
474478
};
475-
static validateIframeAttribute: o.ExternalReference = {
476-
name: 'ɵɵvalidateIframeAttribute',
477-
moduleName: CORE,
478-
};
479479

480480
// Decorators
481481
static inputDecorator: o.ExternalReference = {name: 'Input', moduleName: CORE};

0 commit comments

Comments
 (0)