Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Commit 8616ba9

Browse filesBrowse files
alan-agius4pkozlowski-opensource
authored andcommitted
fix(core): ensure SVG animation attributeName is checked case-insensitively
Currently, the SVG sanitizer checks a static set of candidate attribute names (`attributeName` and `attributename`). This approach misses other case variations (such as `attributenAme` or others), which could potentially bypass sanitization when binding sensitive attributes like `href` on `<set>` or `<animate>` elements. This change retrieves all attribute names of the SVG element, performs a case-insensitive comparison with `'attributename'`, and sanitizes the value if a match is found. (cherry picked from commit 3499a13)
1 parent 90e81e4 commit 8616ba9
Copy full SHA for 8616ba9

2 files changed

+21-32Lines changed: 21 additions & 32 deletions

File tree

Expand file treeCollapse file tree
Open diff view settings
Filter options
Expand file treeCollapse file tree
Open diff view settings
Collapse file

‎packages/core/src/sanitization/sanitization.ts‎

Copy file name to clipboardExpand all lines: packages/core/src/sanitization/sanitization.ts
+5-2Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -277,7 +277,6 @@ function getSanitizer(): Sanitizer | null {
277277
* Set of attributes that are sensitive and should be sanitized.
278278
*/
279279
const SECURITY_SENSITIVE_ATTRIBUTE_NAMES: ReadonlySet<string> = new Set(['href', 'xlink:href']);
280-
const SVG_ANIMATION_ATTRIBUTE_NAME_CANDIDATES = ['attributeName', 'attributename'] as const;
281280

282281
/**
283282
* @remarks Keep this in sync with DOM Security Schema.
@@ -394,7 +393,11 @@ function getSecuritySensitiveSVGAnimationAttributeName(
394393
element: SVGAnimateElement,
395394
validationConfig: ReadonlySet<string>,
396395
): string | null {
397-
for (const attributeName of SVG_ANIMATION_ATTRIBUTE_NAME_CANDIDATES) {
396+
for (const attributeName of element.getAttributeNames()) {
397+
if (attributeName.toLowerCase() !== 'attributename') {
398+
continue;
399+
}
400+
398401
const attributeNameValue = element.getAttribute(attributeName);
399402
if (attributeNameValue !== null && validationConfig.has(attributeNameValue.toLowerCase())) {
400403
return attributeNameValue;
Collapse file

‎packages/core/test/render3/integration_spec.ts‎

Copy file name to clipboardExpand all lines: packages/core/test/render3/integration_spec.ts
+16-30Lines changed: 16 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -662,22 +662,6 @@ describe('sanitization', () => {
662662
);
663663
});
664664

665-
it('should throw when binding to set element with attributeName="href"', () => {
666-
@Component({
667-
selector: 'test-comp',
668-
template: `<svg><set attributeName="href" [to]="'foo'"></set></svg>`,
669-
})
670-
class TestComp {}
671-
672-
TestBed.configureTestingModule({
673-
providers: [provideZoneChangeDetection()],
674-
});
675-
const fixture = TestBed.createComponent(TestComp);
676-
expect(() => fixture.detectChanges()).toThrowError(
677-
/Angular has detected that the `to` was applied/,
678-
);
679-
});
680-
681665
// The SVG `attributeName` is case-sensitive when accessed via the DOM API
682666
// (i.e. `setAttribute('attributename', ...)` and `setAttribute('attributeName', ...)`
683667
// create two distinct attributes). However, the browser tokenizer normalizes
@@ -686,21 +670,23 @@ describe('sanitization', () => {
686670
// The SSR renderer (Domino) does not perform this normalization, so we
687671
// explicitly look up the lowercase form as well to make sure the sanitizer
688672
// is triggered consistently in both environments.
689-
it('should throw when binding to set element with attributename="href"', () => {
690-
@Component({
691-
selector: 'test-comp',
692-
template: `<svg><set attributename="href" [attr.to]="'foo'"></set></svg>`,
693-
})
694-
class TestComp {}
695-
696-
TestBed.configureTestingModule({
697-
providers: [provideZoneChangeDetection()],
673+
for (const attr of ['attributeName', 'attributename', 'attributenAme']) {
674+
it(`should throw when binding to set element with ${attr}="href"`, () => {
675+
@Component({
676+
selector: 'test-comp',
677+
template: `<svg><set ${attr}="href" [attr.to]="'foo'"></set></svg>`,
678+
})
679+
class TestComp {}
680+
681+
TestBed.configureTestingModule({
682+
providers: [provideZoneChangeDetection()],
683+
});
684+
const fixture = TestBed.createComponent(TestComp);
685+
expect(() => fixture.detectChanges()).toThrowError(
686+
/Angular has detected that the `to` was applied/,
687+
);
698688
});
699-
const fixture = TestBed.createComponent(TestComp);
700-
expect(() => fixture.detectChanges()).toThrowError(
701-
/Angular has detected that the `to` was applied/,
702-
);
703-
});
689+
}
704690

705691
it('should not throw when binding to animate element when attributeName is not href', () => {
706692
@Component({

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.