From 7168bed663ba2a53f087a68a251e8ccfd938eb97 Mon Sep 17 00:00:00 2001 From: SkyZeroZx <73321943+SkyZeroZx@users.noreply.github.com> Date: Mon, 31 Aug 2026 14:08:39 -0500 Subject: [PATCH] fix(core): validate SVG animation attributes outside the SVG namespace Make the SVG animation security context depend on the tag name instead of the namespace the element was created in. An animation element declared outside an `` is created in the HTML namespace, but still animates once it ends up inside an SVG subtree, so to and `attributeName` bindings were reaching the DOM unvalidated. Fixes #70490 --- .../src/schema/dom_security_schema.ts | 9 + .../dom_element_schema_registry_spec.ts | 27 +++ .../src/sanitization/dom_security_schema.ts | 9 + .../core/src/sanitization/sanitization.ts | 2 +- .../core/test/render3/integration_spec.ts | 160 ++++++++++++++++++ 5 files changed, 206 insertions(+), 1 deletion(-) diff --git a/packages/compiler/src/schema/dom_security_schema.ts b/packages/compiler/src/schema/dom_security_schema.ts index fbd3c81f136..022caf78684 100644 --- a/packages/compiler/src/schema/dom_security_schema.ts +++ b/packages/compiler/src/schema/dom_security_schema.ts @@ -210,5 +210,14 @@ export function checkSecurityContext( } } + // An SVG animation element declared outside of an `` has no explicit namespace at compile + // time, but can still animate once it is projected into an SVG subtree. + if (context === undefined && (!namespace || namespace === NO_NAMESPACE)) { + const svgSchema = attrSchema[SVG_NAMESPACE]; + if (svgSchema) { + context = svgSchema[tagLower]; + } + } + return context ?? SecurityContext.NONE; } diff --git a/packages/compiler/test/schema/dom_element_schema_registry_spec.ts b/packages/compiler/test/schema/dom_element_schema_registry_spec.ts index d0cd4dbad34..f5258c0988c 100644 --- a/packages/compiler/test/schema/dom_element_schema_registry_spec.ts +++ b/packages/compiler/test/schema/dom_element_schema_registry_spec.ts @@ -172,6 +172,33 @@ If 'onAnything' is a directive input, make sure the directive is imported by the SecurityContext.ATTRIBUTE_NO_BINDING, ); + // SVG animation elements are sensitive when their namespace is omitted at compile time. + expect(registry.securityContext('set', 'to', true)).toBe(SecurityContext.ATTRIBUTE_NO_BINDING); + expect(registry.securityContext('set', 'attributeName', true)).toBe( + SecurityContext.ATTRIBUTE_NO_BINDING, + ); + expect(registry.securityContext('animate', 'to', true)).toBe( + SecurityContext.ATTRIBUTE_NO_BINDING, + ); + expect(registry.securityContext('animate', 'from', true)).toBe( + SecurityContext.ATTRIBUTE_NO_BINDING, + ); + expect(registry.securityContext('animate', 'values', true)).toBe( + SecurityContext.ATTRIBUTE_NO_BINDING, + ); + expect(registry.securityContext('animate', 'attributeName', true)).toBe( + SecurityContext.ATTRIBUTE_NO_BINDING, + ); + expect(registry.securityContext('animateMotion', 'attributeName', true)).toBe( + SecurityContext.ATTRIBUTE_NO_BINDING, + ); + expect(registry.securityContext('animateTransform', 'attributeName', true)).toBe( + SecurityContext.ATTRIBUTE_NO_BINDING, + ); + + // The defensive SVG lookup must not apply to elements in an explicit foreign namespace. + expect(registry.securityContext(':math:set', 'to', true)).toBe(SecurityContext.NONE); + // SVG link attributes expect(registry.securityContext(':svg:a', 'href', false)).toBe(SecurityContext.URL); expect(registry.securityContext(':svg:a', 'xlink:href', false)).toBe(SecurityContext.URL); diff --git a/packages/core/src/sanitization/dom_security_schema.ts b/packages/core/src/sanitization/dom_security_schema.ts index fbd3c81f136..022caf78684 100644 --- a/packages/core/src/sanitization/dom_security_schema.ts +++ b/packages/core/src/sanitization/dom_security_schema.ts @@ -210,5 +210,14 @@ export function checkSecurityContext( } } + // An SVG animation element declared outside of an `` has no explicit namespace at compile + // time, but can still animate once it is projected into an SVG subtree. + if (context === undefined && (!namespace || namespace === NO_NAMESPACE)) { + const svgSchema = attrSchema[SVG_NAMESPACE]; + if (svgSchema) { + context = svgSchema[tagLower]; + } + } + return context ?? SecurityContext.NONE; } diff --git a/packages/core/src/sanitization/sanitization.ts b/packages/core/src/sanitization/sanitization.ts index 4c4fee794ea..f3f8cabec65 100644 --- a/packages/core/src/sanitization/sanitization.ts +++ b/packages/core/src/sanitization/sanitization.ts @@ -358,7 +358,7 @@ export function ɵɵvalidateAttribute(value: T, tagName: string, attrib if (resolvedTagName === 'iframe') { const element = getNativeByTNode(tNode, lView) as RElement; enforceIframeSecurity(element as HTMLIFrameElement); - } else if (namespace === SVG_NAMESPACE) { + } else if (namespace === SVG_NAMESPACE || !namespace) { const config = SVG_ANIMATION_SENSITIVE_STATIC_VALUES[resolvedTagName]?.[attributeName.toLowerCase()]; if (config) { diff --git a/packages/core/test/render3/integration_spec.ts b/packages/core/test/render3/integration_spec.ts index 2423d675470..4fffd3adda7 100644 --- a/packages/core/test/render3/integration_spec.ts +++ b/packages/core/test/render3/integration_spec.ts @@ -10,9 +10,13 @@ import {CommonModule} from '@angular/common'; import { ChangeDetectionStrategy, Component, + createComponent, Directive, + EnvironmentInjector, HostBinding, + inject, provideZoneChangeDetection, + ViewContainerRef, } from '../../src/core'; import {TestBed} from '../../testing'; @@ -688,6 +692,162 @@ describe('sanitization', () => { }); } + // One carrier per attribute: directive metadata has to be statically analyzable, so the bound + // attribute name cannot be interpolated into `host`. + const UNSAFE_VALUE = 'javascript:alert(1)'; + + @Directive({selector: '[bind-to]', host: {'[attr.to]': 'value'}}) + class BindTo { + value = UNSAFE_VALUE; + } + + @Directive({selector: '[bind-from]', host: {'[attr.from]': 'value'}}) + class BindFrom { + value = UNSAFE_VALUE; + } + + @Directive({selector: '[bind-values]', host: {'[attr.values]': 'value'}}) + class BindValues { + value = UNSAFE_VALUE; + } + + @Directive({selector: '[bind-attribute-name]', host: {'[attr.attributeName]': 'value'}}) + class BindAttributeName { + value = UNSAFE_VALUE; + } + + const attrCarriers = { + 'to': BindTo, + 'from': BindFrom, + 'values': BindValues, + 'attributeName': BindAttributeName, + }; + + // Binds one attribute on a host element created in the HTML namespace. Going through a concrete + // `hostElement` keeps the tag name out of the template, which lets the cases below be table + // driven. Returns the change detection call so the caller can assert on it. + function bindAttributeOutsideSvg( + tagName: string, + attrName: keyof typeof attrCarriers, + staticAttributeName: string, + ): () => void { + @Component({ + template: '', + changeDetection: ChangeDetectionStrategy.Eager, + }) + class DynamicHost {} + + const hostElement = document.createElement(tagName); + hostElement.setAttribute('attributeName', staticAttributeName); + + const componentRef = createComponent(DynamicHost, { + hostElement, + environmentInjector: TestBed.inject(EnvironmentInjector), + directives: [attrCarriers[attrName]], + }); + + return () => { + try { + componentRef.changeDetectorRef.detectChanges(); + } finally { + componentRef.destroy(); + } + }; + } + + // An animation element declared outside of an `` is created in the HTML namespace, but still + // animates once it ends up inside an `` subtree - for example when it is projected into one + // and the server-rendered markup is re-parsed by the browser. + const svgAnimationAttrs: Record = { + 'set': ['to', 'attributeName'], + 'animate': ['to', 'from', 'values', 'attributeName'], + 'animateMotion': ['attributeName'], + 'animateTransform': ['attributeName'], + }; + + for (const [tagName, attrNames] of Object.entries(svgAnimationAttrs)) { + for (const attrName of attrNames) { + it(`should throw when binding to \`${attrName}\` on a <${tagName}> outside of an svg element`, () => { + const detectChanges = bindAttributeOutsideSvg(tagName, attrName, 'href'); + + // The tag name is reported as the security check lower-cased it. + expect(detectChanges).toThrowError( + new RegExp( + `Angular has detected that the \`${attrName}\` was applied as a binding ` + + `to the <${tagName.toLowerCase()}> element`, + ), + ); + }); + } + } + + it('should throw when binding to a set element projected into an svg element', () => { + @Component({ + selector: 'svg-wrapper', + template: ` + + + + + + + + `, + }) + class SvgWrapper {} + + @Component({ + selector: 'test-comp', + imports: [SvgWrapper], + template: ` + + + + `, + changeDetection: ChangeDetectionStrategy.Eager, + }) + class TestComp {} + + const fixture = TestBed.createComponent(TestComp); + expect(() => fixture.detectChanges()).toThrowError( + /Angular has detected that the `to` was applied/, + ); + }); + + it('should throw when binding to a dynamically created set component host', () => { + @Component({ + selector: 'set[drilldown]', + template: '', + host: { + 'attributeName': 'href', + '[attr.to]': 'target', + }, + changeDetection: ChangeDetectionStrategy.Eager, + }) + class DynamicSetComp { + target = 'javascript:alert(1)'; + } + + @Component({ + selector: 'test-comp', + template: '', + }) + class TestComp { + readonly viewContainerRef = inject(ViewContainerRef); + } + + const fixture = TestBed.createComponent(TestComp); + fixture.componentInstance.viewContainerRef.createComponent(DynamicSetComp); + + expect(() => fixture.detectChanges()).toThrowError( + /Angular has detected that the `to` was applied/, + ); + }); + + it('should not throw when binding to a set element outside of an svg element when attributeName is not href', () => { + expect(bindAttributeOutsideSvg('set', 'to', 'display')).not.toThrow(); + }); + it('should not throw when binding to animate element when attributeName is not href', () => { @Component({ selector: 'test-comp',