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',