From 6caa298dee58319b2d674dc91364e26ffe3ecb2b Mon Sep 17 00:00:00 2001 From: SkyZeroZx <73321943+SkyZeroZx@users.noreply.github.com> Date: Tue, 30 Jun 2026 16:25:17 -0500 Subject: [PATCH 1/7] fix(core): sanitize host bindings on concrete hosts Host binding sanitization previously used the declaring directive or component selector to choose a compile-time security context. The same host binding can execute on a different concrete element through hostDirectives, inherited host bindings, dynamic directives, or createComponent hostElement usage. Compute host binding security contexts against possible concrete hosts and defer URL versus ResourceURL selection to runtime when necessary. Resolve dynamic root host TNodes to their native tag before sanitizer and security-sensitive attribute checks. Fixes angular#69550 --- .../host_bindings/GOLDEN_PARTIAL.js | 57 +++ .../host_bindings/sanitization.js | 26 +- .../host_bindings/sanitization.ts | 30 ++ .../sanitization_isolated.golden.d.ts | 16 +- .../compiler-cli/test/ngtsc/ngtsc_spec.ts | 42 +- .../src/template/pipeline/src/ingest.ts | 60 ++- .../pipeline/src/phases/resolve_sanitizers.ts | 57 ++- packages/core/src/render3/component_ref.ts | 3 +- packages/core/src/render3/interfaces/node.ts | 8 + .../core/src/sanitization/sanitization.ts | 103 ++++- .../core/test/acceptance/security_spec.ts | 371 ++++++++++++++++++ .../router/bundle.golden_symbols.json | 7 +- .../test/sanitization/sanitization_spec.ts | 20 +- 13 files changed, 747 insertions(+), 53 deletions(-) diff --git a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/host_bindings/GOLDEN_PARTIAL.js b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/host_bindings/GOLDEN_PARTIAL.js index 993805dc69ac..9328636dca1f 100644 --- a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/host_bindings/GOLDEN_PARTIAL.js +++ b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/host_bindings/GOLDEN_PARTIAL.js @@ -981,6 +981,48 @@ i0.ɵɵngDeclareClassMetadata({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDE }, }] }] }); +export class HostBindingCustomSrcdocDir { + evil = 'evil'; + static ɵfac = i0.ɵɵngDeclareFactory({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: HostBindingCustomSrcdocDir, deps: [], target: i0.ɵɵFactoryTarget.Directive }); + static ɵdir = i0.ɵɵngDeclareDirective({ minVersion: "14.0.0", version: "0.0.0-PLACEHOLDER", type: HostBindingCustomSrcdocDir, isStandalone: true, selector: "safe-srcdoc-carrier", host: { properties: { "attr.srcdoc": "evil" } }, ngImport: i0 }); +} +i0.ɵɵngDeclareClassMetadata({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: HostBindingCustomSrcdocDir, decorators: [{ + type: Directive, + args: [{ + selector: 'safe-srcdoc-carrier', + host: { + '[attr.srcdoc]': 'evil', + }, + }] + }] }); +export class HostBindingCustomSrcDir { + evil = 'evil'; + static ɵfac = i0.ɵɵngDeclareFactory({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: HostBindingCustomSrcDir, deps: [], target: i0.ɵɵFactoryTarget.Directive }); + static ɵdir = i0.ɵɵngDeclareDirective({ minVersion: "14.0.0", version: "0.0.0-PLACEHOLDER", type: HostBindingCustomSrcDir, isStandalone: true, selector: "safe-src-carrier", host: { properties: { "attr.src": "evil" } }, ngImport: i0 }); +} +i0.ɵɵngDeclareClassMetadata({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: HostBindingCustomSrcDir, decorators: [{ + type: Directive, + args: [{ + selector: 'safe-src-carrier', + host: { + '[attr.src]': 'evil', + }, + }] + }] }); +export class HostBindingCustomDataDir { + evil = 'evil'; + static ɵfac = i0.ɵɵngDeclareFactory({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: HostBindingCustomDataDir, deps: [], target: i0.ɵɵFactoryTarget.Directive }); + static ɵdir = i0.ɵɵngDeclareDirective({ minVersion: "14.0.0", version: "0.0.0-PLACEHOLDER", type: HostBindingCustomDataDir, isStandalone: true, selector: "safe-data-carrier", host: { properties: { "attr.data": "evil" } }, ngImport: i0 }); +} +i0.ɵɵngDeclareClassMetadata({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: HostBindingCustomDataDir, decorators: [{ + type: Directive, + args: [{ + selector: 'safe-data-carrier', + host: { + '[attr.data]': 'evil', + }, + }] + }] }); /**************************************************************************************************** * PARTIAL FILE: sanitization.d.ts @@ -1008,6 +1050,21 @@ export declare class HostBindingSvgAnimateDir { static ɵfac: i0.ɵɵFactoryDeclaration; static ɵdir: i0.ɵɵDirectiveDeclaration; } +export declare class HostBindingCustomSrcdocDir { + evil: string; + static ɵfac: i0.ɵɵFactoryDeclaration; + static ɵdir: i0.ɵɵDirectiveDeclaration; +} +export declare class HostBindingCustomSrcDir { + evil: string; + static ɵfac: i0.ɵɵFactoryDeclaration; + static ɵdir: i0.ɵɵDirectiveDeclaration; +} +export declare class HostBindingCustomDataDir { + evil: string; + static ɵfac: i0.ɵɵFactoryDeclaration; + static ɵdir: i0.ɵɵDirectiveDeclaration; +} /**************************************************************************************************** * PARTIAL FILE: security_sensitive_constant_attributes.js diff --git a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/host_bindings/sanitization.js b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/host_bindings/sanitization.js index 9ea142fafedc..073c1eac4008 100644 --- a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/host_bindings/sanitization.js +++ b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/host_bindings/sanitization.js @@ -1,21 +1,21 @@ hostBindings: function HostBindingLinkDir_HostBindings(rf, ctx) { if (rf & 2) { - $r3$.ɵɵdomProperty("innerHTML", ctx.evil, $r3$.ɵɵsanitizeHtml)("href", ctx.evil, $r3$.ɵɵsanitizeUrl); + $r3$.ɵɵdomProperty("innerHTML", ctx.evil, $r3$.ɵɵsanitizeHtml)("href", ctx.evil, $r3$.ɵɵsanitizeUrlOrResourceUrl); $r3$.ɵɵattribute("style", ctx.evil, $r3$.ɵɵsanitizeStyle); } } … hostBindings: function HostBindingImageDir_HostBindings(rf, ctx) { if (rf & 2) { - i0.ɵɵdomProperty("innerHTML", ctx.evil, i0.ɵɵsanitizeHtml)("src", ctx.nonEvil, i0.ɵɵsanitizeUrl); + i0.ɵɵdomProperty("innerHTML", ctx.evil, i0.ɵɵsanitizeHtml)("src", ctx.nonEvil, i0.ɵɵsanitizeUrlOrResourceUrl); i0.ɵɵattribute("style", ctx.evil, i0.ɵɵsanitizeStyle); } } … hostBindings: function HostBindingIframeDir_HostBindings(rf, ctx) { if (rf & 2) { - $r3$.ɵɵdomProperty("innerHTML", ctx.evil, $r3$.ɵɵsanitizeHtml)("src", ctx.evil, i0.ɵɵsanitizeResourceUrl)("sandbox", ctx.evil, $r3$.ɵɵvalidateAttribute); - $r3$.ɵɵattribute("style", ctx.evil, $r3$.ɵɵsanitizeStyle)("attributeName", ctx.nonEvil); + $r3$.ɵɵdomProperty("innerHTML", ctx.evil, $r3$.ɵɵsanitizeHtml)("src", ctx.evil, i0.ɵɵsanitizeUrlOrResourceUrl)("sandbox", ctx.evil, $r3$.ɵɵvalidateAttribute); + $r3$.ɵɵattribute("style", ctx.evil, $r3$.ɵɵsanitizeStyle)("attributeName", ctx.nonEvil, i0.ɵɵvalidateAttribute); } } … @@ -24,3 +24,21 @@ hostBindings: function HostBindingSvgAnimateDir_HostBindings(rf, ctx) { i0.ɵɵattribute("attributeName", ctx.evil, i0.ɵɵvalidateAttribute); } } +… +hostBindings: function HostBindingCustomSrcdocDir_HostBindings(rf, ctx) { + if (rf & 2) { + i0.ɵɵattribute("srcdoc", ctx.evil, i0.ɵɵsanitizeHtml); + } +} +… +hostBindings: function HostBindingCustomSrcDir_HostBindings(rf, ctx) { + if (rf & 2) { + i0.ɵɵattribute("src", ctx.evil, i0.ɵɵsanitizeUrlOrResourceUrl); + } +} +… +hostBindings: function HostBindingCustomDataDir_HostBindings(rf, ctx) { + if (rf & 2) { + i0.ɵɵattribute("data", ctx.evil, i0.ɵɵsanitizeUrlOrResourceUrl); + } +} diff --git a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/host_bindings/sanitization.ts b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/host_bindings/sanitization.ts index 92a17ab2e3f5..54669e19872f 100644 --- a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/host_bindings/sanitization.ts +++ b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/host_bindings/sanitization.ts @@ -49,3 +49,33 @@ export class HostBindingIframeDir { export class HostBindingSvgAnimateDir { evil = 'evil'; } + +@Directive({ + selector: 'safe-srcdoc-carrier', + host: { + '[attr.srcdoc]': 'evil', + }, +}) +export class HostBindingCustomSrcdocDir { + evil = 'evil'; +} + +@Directive({ + selector: 'safe-src-carrier', + host: { + '[attr.src]': 'evil', + }, +}) +export class HostBindingCustomSrcDir { + evil = 'evil'; +} + +@Directive({ + selector: 'safe-data-carrier', + host: { + '[attr.data]': 'evil', + }, +}) +export class HostBindingCustomDataDir { + evil = 'evil'; +} diff --git a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/host_bindings/sanitization_isolated.golden.d.ts b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/host_bindings/sanitization_isolated.golden.d.ts index f0fae600478d..8ee2476afe40 100644 --- a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/host_bindings/sanitization_isolated.golden.d.ts +++ b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/host_bindings/sanitization_isolated.golden.d.ts @@ -21,4 +21,18 @@ export declare class HostBindingSvgAnimateDir { static ɵfac: i0.ɵɵFactoryDeclaration; static ɵdir: i0.ɵɵDirectiveDeclaration; } - +export declare class HostBindingCustomSrcdocDir { + evil: string; + static ɵfac: i0.ɵɵFactoryDeclaration; + static ɵdir: i0.ɵɵDirectiveDeclaration; +} +export declare class HostBindingCustomSrcDir { + evil: string; + static ɵfac: i0.ɵɵFactoryDeclaration; + static ɵdir: i0.ɵɵDirectiveDeclaration; +} +export declare class HostBindingCustomDataDir { + evil: string; + static ɵfac: i0.ɵɵFactoryDeclaration; + static ɵdir: i0.ɵɵDirectiveDeclaration; +} diff --git a/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts b/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts index e8d53ab0c475..291cbd11754b 100644 --- a/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts +++ b/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts @@ -8614,7 +8614,7 @@ runInEachFileSystem((os: string) => { hostVars: 6, hostBindings: function UnsafeAttrsDirective_HostBindings(rf, ctx) { if (rf & 2) { - i0.ɵɵattribute("href", ctx.attrHref, i0.ɵɵsanitizeUrlOrResourceUrl)("src", ctx.attrSrc, i0.ɵɵsanitizeUrlOrResourceUrl)("action", ctx.attrAction, i0.ɵɵsanitizeUrl)("profile", ctx.attrProfile)("innerHTML", ctx.attrInnerHTML, i0.ɵɵsanitizeHtml)("title", ctx.attrSafeTitle); + i0.ɵɵattribute("href", ctx.attrHref, i0.ɵɵsanitizeUrlOrResourceUrl)("src", ctx.attrSrc, i0.ɵɵsanitizeUrlOrResourceUrl)("action", ctx.attrAction, i0.ɵɵsanitizeUrlOrResourceUrl)("profile", ctx.attrProfile)("innerHTML", ctx.attrInnerHTML, i0.ɵɵsanitizeHtml)("title", ctx.attrSafeTitle); } } `; @@ -8660,14 +8660,14 @@ runInEachFileSystem((os: string) => { hostVars: 3, hostBindings: function UnsafePropsDirective_HostBindings(rf, ctx) { if (rf & 2) { - i0.ɵɵdomProperty("href", ctx.propHref, i0.ɵɵsanitizeUrl)("innerHTML", ctx.propInnerHTML, i0.ɵɵsanitizeHtml)("title", ctx.propSafeTitle); + i0.ɵɵdomProperty("href", ctx.propHref, i0.ɵɵsanitizeUrlOrResourceUrl)("innerHTML", ctx.propInnerHTML, i0.ɵɵsanitizeHtml)("title", ctx.propSafeTitle); } } `; expect(trim(jsContents)).toContain(trim(hostBindingsFn)); }); - it('should not generate sanitizers for URL properties in hostBindings fn in Component', () => { + it('should generate concrete-host URL sanitizers in hostBindings fn in Component', () => { env.write( `test.ts`, ` @@ -8700,8 +8700,40 @@ runInEachFileSystem((os: string) => { hostVars: 5, hostBindings: function FooCmp_HostBindings(rf, ctx) { if (rf & 2) { - i0.ɵɵdomProperty("href", ctx.hrefProp, i0.ɵɵsanitizeUrl)("title", ctx.titleProp); - i0.ɵɵattribute("src", ctx.srcAttr)("href", ctx.hrefAttr, i0.ɵɵsanitizeUrl)("title", ctx.titleAttr); + i0.ɵɵdomProperty("href", ctx.hrefProp, i0.ɵɵsanitizeUrlOrResourceUrl)("title", ctx.titleProp); + i0.ɵɵattribute("src", ctx.srcAttr, i0.ɵɵsanitizeUrlOrResourceUrl)("href", ctx.hrefAttr, i0.ɵɵsanitizeUrlOrResourceUrl)("title", ctx.titleAttr); + } + } + `; + expect(trim(jsContents)).toContain(trim(hostBindingsFn)); + }); + + it('should generate sanitizers for pure :not selector host bindings', () => { + env.write( + `test.ts`, + ` + import {Component} from '@angular/core'; + + @Component({ + selector: ':not(iframe)', + template: '', + host: { + '[attr.srcdoc]': 'srcdoc', + } + }) + class FooCmp { + srcdoc: any; + } + `, + ); + + env.driveMain(); + const jsContents = env.getContents('test.js'); + const hostBindingsFn = ` + hostVars: 1, + hostBindings: function FooCmp_HostBindings(rf, ctx) { + if (rf & 2) { + i0.ɵɵattribute("srcdoc", ctx.srcdoc, i0.ɵɵsanitizeHtml); } } `; diff --git a/packages/compiler/src/template/pipeline/src/ingest.ts b/packages/compiler/src/template/pipeline/src/ingest.ts index 9b63a8584191..db2b2069a630 100644 --- a/packages/compiler/src/template/pipeline/src/ingest.ts +++ b/packages/compiler/src/template/pipeline/src/ingest.ts @@ -21,7 +21,7 @@ import { } from '../../../render3/view/api'; import {icuFromI18nMessage} from '../../../render3/view/i18n/util'; import {DomElementSchemaRegistry} from '../../../schema/dom_element_schema_registry'; -import {BindingParser} from '../../../template_parser/binding_parser'; +import {BindingParser, calcPossibleSecurityContexts} from '../../../template_parser/binding_parser'; import * as ir from '../ir'; import { @@ -125,19 +125,21 @@ export function ingestHostBinding( if (property.isAnimation) { bindingKind = ir.BindingKind.Animation; } - const securityContexts = bindingParser - .calcPossibleSecurityContexts( - input.componentSelector, - property.name, - bindingKind === ir.BindingKind.Attribute, - ) - .filter((context) => context !== SecurityContext.NONE); + const securityContexts = calcHostBindingSecurityContexts( + bindingParser, + input.componentSelector, + property.name, + bindingKind === ir.BindingKind.Attribute, + ); ingestDomProperty(job, property, bindingKind, securityContexts); } for (const [name, expr] of Object.entries(input.attributes) ?? []) { - const securityContexts = bindingParser - .calcPossibleSecurityContexts(input.componentSelector, name, true) - .filter((context) => context !== SecurityContext.NONE); + const securityContexts = calcHostBindingSecurityContexts( + bindingParser, + input.componentSelector, + name, + true, + ); ingestHostAttribute(job, name, expr, securityContexts); } for (const event of input.events ?? []) { @@ -146,6 +148,42 @@ export function ingestHostBinding( return job; } +function calcHostBindingSecurityContexts( + bindingParser: BindingParser, + selector: string, + name: string, + isAttribute: boolean, +): SecurityContext[] { + const declaringSelectorContexts = bindingParser.calcPossibleSecurityContexts( + selector, + name, + isAttribute, + ); + const concreteHostContexts = calcPossibleSecurityContexts( + domSchema, + null, + domSchema.getMappedPropName(name), + isAttribute, + ); + const concreteHostNonNoneContexts = concreteHostContexts.filter( + (context) => context !== SecurityContext.NONE, + ); + const concreteHostNonNoneCount = concreteHostNonNoneContexts.length; + const hasConcreteHostNoneContext = concreteHostNonNoneCount !== concreteHostContexts.length; + + // Host bindings can run against a concrete host whose element name differs from the declaring + // selector, including dynamic root components whose TNode name is `#host`. + if (hasConcreteHostNoneContext && concreteHostNonNoneCount > 0) { + return concreteHostContexts; + } + + if (concreteHostNonNoneContexts.some((context) => !declaringSelectorContexts.includes(context))) { + return concreteHostContexts; + } + + return declaringSelectorContexts.filter((context) => context !== SecurityContext.NONE); +} + // TODO: We should refactor the parser to use the same types and structures for host bindings as // with ordinary components. This would allow us to share a lot more ingestion code. export function ingestDomProperty( diff --git a/packages/compiler/src/template/pipeline/src/phases/resolve_sanitizers.ts b/packages/compiler/src/template/pipeline/src/phases/resolve_sanitizers.ts index c1b9ba3c6f44..1012a3beefae 100644 --- a/packages/compiler/src/template/pipeline/src/phases/resolve_sanitizers.ts +++ b/packages/compiler/src/template/pipeline/src/phases/resolve_sanitizers.ts @@ -58,16 +58,11 @@ export function resolveSanitizers(job: CompilationJob): void { case ir.OpKind.DomProperty: case ir.OpKind.TwoWayProperty: let sanitizerFn: o.ExternalReference | null = null; - if ( - Array.isArray(op.securityContext) && - op.securityContext.length === 2 && - op.securityContext.includes(SecurityContext.URL) && - op.securityContext.includes(SecurityContext.RESOURCE_URL) - ) { - // When the host element isn't known, some URL attributes (such as "src" and "href") may - // be part of multiple different security contexts. In this case we use special - // sanitization function and select the actual sanitizer at runtime based on a tag name - // that is provided while invoking sanitization function. + if (isUrlOrResourceUrlSecurityContext(op.securityContext)) { + // When the host element isn't known, attributes such as `href`, `src`, `data`, + // `action`, and `codebase` may be part of multiple security contexts. In this case we + // use a special sanitization function and select the actual behavior at runtime based + // on the concrete host element. sanitizerFn = Identifiers.sanitizeUrlOrResourceUrl; } else { sanitizerFn = sanitizerFns.get(getOnlySecurityContext(op.securityContext)) ?? null; @@ -81,21 +76,57 @@ export function resolveSanitizers(job: CompilationJob): void { } } +function isUrlOrResourceUrlSecurityContext( + securityContext: SecurityContext | SecurityContext[], +): boolean { + if (!Array.isArray(securityContext)) { + return false; + } + + let hasUrlContext = false; + let hasResourceUrlContext = false; + let hasNoneContext = false; + + for (const context of securityContext) { + switch (context) { + case SecurityContext.URL: + hasUrlContext = true; + break; + case SecurityContext.RESOURCE_URL: + hasResourceUrlContext = true; + break; + case SecurityContext.NONE: + hasNoneContext = true; + break; + default: + return false; + } + } + + return ( + ((hasUrlContext || hasResourceUrlContext) && hasNoneContext) || + (hasUrlContext && hasResourceUrlContext) + ); +} + /** - * Asserts that there is only a single security context and returns it. + * Asserts that there is only a single non-NONE security context and returns it. */ function getOnlySecurityContext( securityContext: SecurityContext | SecurityContext[], ): SecurityContext { if (Array.isArray(securityContext)) { - if (securityContext.length > 1) { + const nonNoneSecurityContexts = securityContext.filter( + (context) => context !== SecurityContext.NONE, + ); + if (nonNoneSecurityContexts.length > 1) { // TODO: What should we do here? TDB just took the first one, but this feels like something we // would want to know about and create a special case for like we did for Url/ResourceUrl. My // guess is that, outside of the Url/ResourceUrl case, this never actually happens. If there // do turn out to be other cases, throwing an error until we can address it feels safer. throw Error(`AssertionError: Ambiguous security context`); } - return securityContext[0] || SecurityContext.NONE; + return nonNoneSecurityContexts[0] || SecurityContext.NONE; } return securityContext; } diff --git a/packages/core/src/render3/component_ref.ts b/packages/core/src/render3/component_ref.ts index 71c5056cc0de..50e10b53d058 100644 --- a/packages/core/src/render3/component_ref.ts +++ b/packages/core/src/render3/component_ref.ts @@ -42,6 +42,7 @@ import { TElementContainerNode, TElementNode, TNode, + TNodeName, TNodeType, } from './interfaces/node'; import {RElement, RNode} from './interfaces/renderer_dom'; @@ -369,7 +370,7 @@ export class ComponentFactory { HEADER_OFFSET, rootLView, TNodeType.Element, - '#host', + TNodeName.DynamicHost, () => rootTView.directiveRegistry, true, 0, diff --git a/packages/core/src/render3/interfaces/node.ts b/packages/core/src/render3/interfaces/node.ts index d6cca96f020b..74898f95f7d7 100644 --- a/packages/core/src/render3/interfaces/node.ts +++ b/packages/core/src/render3/interfaces/node.ts @@ -16,6 +16,14 @@ import {CssSelector} from './projection'; import {RNode} from './renderer_dom'; import type {LView, TView} from './view'; +/** + * Internal tag name used for a root host `TNode` when Angular creates a component against an + * existing host element. The concrete DOM tag is resolved from the native element at runtime. + */ +export const enum TNodeName { + DynamicHost = '#host', +} + /** * TNodeType corresponds to the {@link TNode} `type` property. * diff --git a/packages/core/src/sanitization/sanitization.ts b/packages/core/src/sanitization/sanitization.ts index 253b77da016b..f1c969c06cd5 100644 --- a/packages/core/src/sanitization/sanitization.ts +++ b/packages/core/src/sanitization/sanitization.ts @@ -10,7 +10,7 @@ import {XSS_SECURITY_URL} from '../error_details_base_url'; import {RuntimeError, RuntimeErrorCode} from '../errors'; import {getTemplateLocationDetails} from '../render3/instructions/element_validation'; import {getDocument} from '../render3/interfaces/document'; -import {TNode, TNodeType} from '../render3/interfaces/node'; +import {TNode, TNodeName, TNodeType} from '../render3/interfaces/node'; import {RElement} from '../render3/interfaces/renderer_dom'; import {ENVIRONMENT} from '../render3/interfaces/view'; import {getLView, getSelectedIndex, getSelectedTNode} from '../render3/state'; @@ -46,7 +46,19 @@ import {_sanitizeUrl} from './url_sanitizer'; * * @codeGenApi */ -export function ɵɵsanitizeHtml(unsafeHtml: any): TrustedHTML | string { +export function ɵɵsanitizeHtml( + unsafeHtml: any, + tagName?: string, + propName?: string, +): TrustedHTML | string { + if ( + tagName !== undefined && + propName !== undefined && + getSecurityContext(tagName, propName) !== SecurityContext.HTML + ) { + return unsafeHtml; + } + const sanitizer = getSanitizer(); if (sanitizer) { return trustedHTMLFromStringBypass(sanitizer.sanitize(SecurityContext.HTML, unsafeHtml) || ''); @@ -213,7 +225,20 @@ export function ɵɵtrustConstantResourceUrl(url: TemplateStringsArray): Trusted return trustedScriptURLFromString(url[0]); } -// Define sets outside the function for O(1) lookups and memory efficiency +const HTML_MAP: Record | undefined> = { + '*': {'innerhtml': true, 'outerhtml': true}, + 'iframe': {'srcdoc': true}, +}; + +const URL_MAP: Record | undefined> = { + '*': {'formaction': true}, + 'area': {'href': true}, + 'a': {'href': true, 'xlink:href': true}, + 'form': {'action': true}, + 'img': {'src': true}, + 'video': {'src': true}, +}; + const RESOURCE_MAP: Record | undefined> = { 'embed': {'src': true}, 'frame': {'src': true}, @@ -228,14 +253,19 @@ const RESOURCE_MAP: Record | undefined> /** * Detects which sanitizer to use for URL property, based on tag name and prop name. * - * The rules are based on the RESOURCE_URL context config from + * The rules are based on the URL and RESOURCE_URL context config from * `packages/compiler/src/schema/dom_security_schema.ts`. - * If tag and prop names don't match Resource URL schema, use URL sanitizer. + * If tag and prop names don't match URL or Resource URL schema, no sanitizer is required. */ export function getUrlSanitizer(tag: string, prop: string) { - const isResource = RESOURCE_MAP[tag.toLowerCase()]?.[prop.toLowerCase()] === true; - - return isResource ? ɵɵsanitizeResourceUrl : ɵɵsanitizeUrl; + switch (getSecurityContext(tag, prop)) { + case SecurityContext.RESOURCE_URL: + return ɵɵsanitizeResourceUrl; + case SecurityContext.URL: + return ɵɵsanitizeUrl; + default: + return null; + } } /** @@ -254,7 +284,52 @@ export function getUrlSanitizer(tag: string, prop: string) { * @codeGenApi */ export function ɵɵsanitizeUrlOrResourceUrl(unsafeUrl: any, tag: string, prop: string): any { - return getUrlSanitizer(tag, prop)(unsafeUrl); + const sanitizer = getUrlSanitizer(tag, prop); + return sanitizer === null ? unsafeUrl : sanitizer(unsafeUrl); +} + +function getSecurityContext(tagName: string, propName: string): SecurityContext { + tagName = resolveHostTagName(tagName).toLowerCase(); + propName = propName.toLowerCase(); + + if (hasSecurityContext(RESOURCE_MAP, tagName, propName)) { + return SecurityContext.RESOURCE_URL; + } + + if (hasSecurityContext(URL_MAP, tagName, propName)) { + return SecurityContext.URL; + } + + if (hasSecurityContext(HTML_MAP, tagName, propName)) { + return SecurityContext.HTML; + } + + return SecurityContext.NONE; +} + +function hasSecurityContext( + map: Record | undefined>, + tagName: string, + propName: string, +): boolean { + return map[tagName]?.[propName] === true || map['*']?.[propName] === true; +} + +function resolveHostTagName(tagName: string): string { + if (tagName !== TNodeName.DynamicHost) { + return tagName; + } + + const index = getSelectedIndex(); + const tNode = index === -1 ? null : getSelectedTNode(); + if (tNode !== null && tNode.type & TNodeType.Element) { + const element = getNativeByTNode(tNode, getLView()) as RElement; + if (element.tagName) { + return element.tagName.toLowerCase(); + } + } + + return tagName; } export function validateAgainstEventProperties(name: string) { @@ -314,15 +389,19 @@ const SECURITY_SENSITIVE_ELEMENTS: Record< * @param attributeName The name of the attribute. */ export function ɵɵvalidateAttribute(value: T, tagName: string, attributeName: string): T { - const lowerCaseTagName = tagName.toLowerCase(); - const lowerCaseAttrName = attributeName.toLowerCase(); - const index = getSelectedIndex(); const tNode: TNode | null = index === -1 ? null : getSelectedTNode(); if (tNode && tNode.type !== TNodeType.Element) { return value; } + if (tagName === TNodeName.DynamicHost && tNode !== null) { + tagName = ((getNativeByTNode(tNode, getLView()) as RElement).tagName || tagName).toLowerCase(); + } + + const lowerCaseTagName = tagName.toLowerCase(); + const lowerCaseAttrName = attributeName.toLowerCase(); + // Leverage tNode.namespace if active, otherwise check both namespaced and base variants. const fullTagName = lowerCaseTagName[0] !== ':' && tNode?.namespace diff --git a/packages/core/test/acceptance/security_spec.ts b/packages/core/test/acceptance/security_spec.ts index a317064ac1bb..0651a362d56b 100644 --- a/packages/core/test/acceptance/security_spec.ts +++ b/packages/core/test/acceptance/security_spec.ts @@ -9,11 +9,15 @@ import {NgIf} from '@angular/common'; import {DomSanitizer} from '@angular/platform-browser'; import { + ApplicationRef, Component, + ComponentRef, createComponent, Directive, EnvironmentInjector, inject, + inputBinding, + Input, provideZoneChangeDetection, TemplateRef, Type, @@ -872,6 +876,373 @@ describe('innerHTML processing', () => { }); }); +describe('host binding sanitization', () => { + const HOST_BINDING_URL = 'http://server/asset'; + const HOST_BINDING_UNSAFE_URL = 'javascript:custom-data'; + const UNSAFE_HTML = `` + '

safe

'; + const SANITIZED_HTML = '

safe

'; + const resourceUrlError = /NG0904: unsafe value used in a resource URL context.*/; + let hostBindingValue = ''; + + @Component({ + selector: 'dynamic-host', + template: '', + }) + class DynamicHostComponent {} + + @Directive({ + selector: 'safe-data-carrier', + host: {'[attr.data]': 'url'}, + }) + class DataCarrierDirective { + url = hostBindingValue; + } + + @Component({ + selector: 'host-srcdoc-carrier', + template: '', + host: {'[attr.srcdoc]': 'srcdoc'}, + }) + class SrcdocHostComponent { + srcdoc = hostBindingValue; + } + + @Component({ + selector: 'host-action-carrier', + template: '', + host: {'[attr.action]': 'action'}, + }) + class ActionHostComponent { + action = hostBindingValue; + } + + let dynamicHostElement: Element; + let dynamicHostDirective: Type; + + @Component({ + template: '', + }) + class DynamicHostTestApp { + componentRef: ComponentRef; + + private appRef = inject(ApplicationRef); + private environmentInjector = inject(EnvironmentInjector); + + constructor() { + this.componentRef = createComponent(DynamicHostComponent, { + hostElement: dynamicHostElement, + environmentInjector: this.environmentInjector, + directives: [dynamicHostDirective], + }); + this.appRef.attachView(this.componentRef.hostView); + } + } + + async function expectDynamicHostAttribute( + tagName: string, + attrName: string, + value: string, + expected: string, + ): Promise { + hostBindingValue = value; + dynamicHostElement = document.createElement(tagName); + dynamicHostDirective = DataCarrierDirective; + const fixture = TestBed.createComponent(DynamicHostTestApp); + + try { + await fixture.whenStable(); + expect(dynamicHostElement.getAttribute(attrName)).toBe(expected); + } finally { + fixture.componentInstance.componentRef.destroy(); + } + } + + async function expectDynamicHostResourceUrlRejection( + tagName: string, + value: string, + ): Promise { + hostBindingValue = value; + dynamicHostElement = document.createElement(tagName); + dynamicHostDirective = DataCarrierDirective; + const fixture = TestBed.createComponent(DynamicHostTestApp); + + try { + await expectAsync(fixture.whenStable()).toBeRejectedWithError(resourceUrlError); + } finally { + fixture.componentInstance.componentRef.destroy(); + } + } + + async function expectComponentHostAttribute( + type: Type, + tagName: string, + attrName: string, + value: string, + expected: string, + ): Promise { + hostBindingValue = value; + const hostElement = document.createElement(tagName); + const appRef = TestBed.inject(ApplicationRef); + const componentRef = createComponent(type, { + hostElement, + environmentInjector: TestBed.inject(EnvironmentInjector), + }); + + try { + appRef.attachView(componentRef.hostView); + await appRef.whenStable(); + + expect(hostElement.getAttribute(attrName)).toBe(expected); + } finally { + componentRef.destroy(); + } + } + + it('should not sanitize resource URL attribute names on non-resource concrete hosts', async () => { + await expectDynamicHostAttribute('div', 'data', HOST_BINDING_URL, HOST_BINDING_URL); + await expectDynamicHostAttribute( + 'div', + 'data', + HOST_BINDING_UNSAFE_URL, + HOST_BINDING_UNSAFE_URL, + ); + }); + + it('should sanitize a dynamic directive host binding against the concrete host element', async () => { + @Component({ + selector: 'iframe', + template: '', + }) + class DynamicIframeHostComponent {} + + @Directive({ + selector: 'safe-srcdoc-carrier', + host: {'[attr.srcdoc]': 'srcdoc'}, + }) + class SafeSrcdocCarrierDirective { + @Input() srcdoc = ''; + } + + @Component({ + template: '', + imports: [DynamicIframeHostComponent], + }) + class App { + componentRef: ComponentRef; + + private viewContainerRef = inject(ViewContainerRef); + private environmentInjector = inject(EnvironmentInjector); + + constructor() { + this.componentRef = this.viewContainerRef.createComponent(DynamicIframeHostComponent, { + environmentInjector: this.environmentInjector, + directives: [ + { + type: SafeSrcdocCarrierDirective, + bindings: [inputBinding('srcdoc', () => UNSAFE_HTML)], + }, + ], + }); + } + } + + const fixture = TestBed.createComponent(App); + await fixture.whenStable(); + + const iframe = fixture.componentInstance.componentRef.location + .nativeElement as HTMLIFrameElement; + expect(iframe.getAttribute('srcdoc')).toBe(SANITIZED_HTML); + expect(iframe.getAttribute('srcdoc')).not.toContain('

safe

'; + + expect(ɵɵsanitizeHtml(html, 'div', 'srcdoc')).toBe(html); + expect(ɵɵsanitizeHtml(html, 'iframe', 'srcdoc').toString()).toBe('

safe

'); + }); + it('should sanitize url', () => { expect(ɵɵsanitizeUrl('http://server')).toEqual('http://server'); expect(ɵɵsanitizeUrl(new Wrap('http://server'))).toEqual('http://server'); @@ -119,6 +126,10 @@ describe('sanitization', () => { for (const [prop, nsSchema] of Object.entries(schema)) { for (const [ns, tagSchema] of Object.entries(nsSchema)) { + if (ns !== '') { + continue; + } + for (const [tag, context] of Object.entries(tagSchema)) { if (context !== SecurityContext.URL && context !== SecurityContext.RESOURCE_URL) { continue; @@ -143,7 +154,8 @@ describe('sanitization', () => { expect(getUrlSanitizer('IFRAME', 'SRC')).toEqual(ɵɵsanitizeResourceUrl); expect(getUrlSanitizer('IFRAME', 'src')).toEqual(ɵɵsanitizeResourceUrl); expect(getUrlSanitizer('iframe', 'SRC')).toEqual(ɵɵsanitizeResourceUrl); - expect(getUrlSanitizer('ScRiPt', 'xLiNk:HrEf')).toEqual(ɵɵsanitizeUrl); + + expect(getUrlSanitizer('DiV', 'DaTa')).toBeNull(); expect(getUrlSanitizer('A', 'HREF')).toEqual(ɵɵsanitizeUrl); }); @@ -156,10 +168,6 @@ describe('sanitization', () => { expect(() => ɵɵsanitizeUrlOrResourceUrl('http://server', 'iframe', 'SRC')).toThrowError(ERROR); - expect(ɵɵsanitizeUrlOrResourceUrl('javascript:true', 'ScRiPt', 'xLiNk:HrEf')).toEqual( - 'unsafe:javascript:true', - ); - expect(ɵɵsanitizeUrlOrResourceUrl('javascript:true', 'A', 'HREF')).toEqual( 'unsafe:javascript:true', ); @@ -200,6 +208,8 @@ describe('sanitization', () => { expect( ɵɵsanitizeUrlOrResourceUrl(bypassSanitizationTrustUrl('javascript:true'), 'a', 'href'), ).toEqual('javascript:true'); + + expect(ɵɵsanitizeUrlOrResourceUrl('javascript:true', 'div', 'data')).toBe('javascript:true'); }); it('should only trust constant strings from template literal tags without interpolation', () => { From fef7173f16f917c673783b1ba75c31af03b791af Mon Sep 17 00:00:00 2001 From: SkyZeroZx <73321943+SkyZeroZx@users.noreply.github.com> Date: Tue, 30 Jun 2026 19:15:14 -0500 Subject: [PATCH 2/7] fix(core): account for namespaces in host binding sanitization Make runtime URL sanitizer selection namespace-aware so SVG and MathML host bindings match the security schema. Cover SVG href/xlink:href and MathML href host binding cases, including dynamic hostElement resolution. --- .../core/src/sanitization/sanitization.ts | 141 +++++++++++++----- .../core/test/acceptance/security_spec.ts | 112 +++++++++++++- .../bundle.golden_symbols.json | 7 + .../bundle.golden_symbols.json | 7 + .../bundling/defer/bundle.golden_symbols.json | 7 + .../forms_reactive/bundle.golden_symbols.json | 7 + .../bundle.golden_symbols.json | 7 + .../hydration/bundle.golden_symbols.json | 7 + .../router/bundle.golden_symbols.json | 14 +- .../bundle.golden_symbols.json | 7 + .../test/sanitization/sanitization_spec.ts | 31 ++-- 11 files changed, 290 insertions(+), 57 deletions(-) diff --git a/packages/core/src/sanitization/sanitization.ts b/packages/core/src/sanitization/sanitization.ts index f1c969c06cd5..7334807863e6 100644 --- a/packages/core/src/sanitization/sanitization.ts +++ b/packages/core/src/sanitization/sanitization.ts @@ -225,29 +225,51 @@ export function ɵɵtrustConstantResourceUrl(url: TemplateStringsArray): Trusted return trustedScriptURLFromString(url[0]); } -const HTML_MAP: Record | undefined> = { - '*': {'innerhtml': true, 'outerhtml': true}, - 'iframe': {'srcdoc': true}, +type SecurityContextMap = Record | undefined>; +type NamespacedSecurityContextMap = Record; + +const NO_NAMESPACE = ''; +const MATCH_ALL_ELEMENTS = '*'; +const SVG_NAMESPACE = 'svg'; +const MATH_ML_NAMESPACE = 'math'; +const SVG_NAMESPACE_URI = 'http://www.w3.org/2000/svg'; +const MATH_ML_NAMESPACE_URI = 'http://www.w3.org/1998/math/mathml'; + +const HTML_MAP: NamespacedSecurityContextMap = { + [NO_NAMESPACE]: { + [MATCH_ALL_ELEMENTS]: {'innerhtml': true, 'outerhtml': true}, + 'iframe': {'srcdoc': true}, + }, }; -const URL_MAP: Record | undefined> = { - '*': {'formaction': true}, - 'area': {'href': true}, - 'a': {'href': true, 'xlink:href': true}, - 'form': {'action': true}, - 'img': {'src': true}, - 'video': {'src': true}, +const URL_MAP: NamespacedSecurityContextMap = { + [NO_NAMESPACE]: { + [MATCH_ALL_ELEMENTS]: {'formaction': true}, + 'area': {'href': true}, + 'a': {'href': true, 'xlink:href': true}, + 'form': {'action': true}, + 'img': {'src': true}, + 'video': {'src': true}, + }, + [MATH_ML_NAMESPACE]: { + [MATCH_ALL_ELEMENTS]: {'href': true, 'xlink:href': true}, + }, + [SVG_NAMESPACE]: { + 'a': {'href': true, 'xlink:href': true}, + }, }; -const RESOURCE_MAP: Record | undefined> = { - 'embed': {'src': true}, - 'frame': {'src': true}, - 'iframe': {'src': true}, - 'media': {'src': true}, +const RESOURCE_MAP: NamespacedSecurityContextMap = { + [NO_NAMESPACE]: { + 'embed': {'src': true}, + 'frame': {'src': true}, + 'iframe': {'src': true}, + 'media': {'src': true}, - 'base': {'href': true}, - 'link': {'href': true}, - 'object': {'data': true, 'codebase': true}, + 'base': {'href': true}, + 'link': {'href': true}, + 'object': {'data': true, 'codebase': true}, + }, }; /** @@ -257,8 +279,8 @@ const RESOURCE_MAP: Record | undefined> * `packages/compiler/src/schema/dom_security_schema.ts`. * If tag and prop names don't match URL or Resource URL schema, no sanitizer is required. */ -export function getUrlSanitizer(tag: string, prop: string) { - switch (getSecurityContext(tag, prop)) { +export function getUrlSanitizer(tag: string, prop: string, elementNamespace?: string | null) { + switch (getSecurityContext(tag, prop, elementNamespace)) { case SecurityContext.RESOURCE_URL: return ɵɵsanitizeResourceUrl; case SecurityContext.URL: @@ -288,48 +310,95 @@ export function ɵɵsanitizeUrlOrResourceUrl(unsafeUrl: any, tag: string, prop: return sanitizer === null ? unsafeUrl : sanitizer(unsafeUrl); } -function getSecurityContext(tagName: string, propName: string): SecurityContext { - tagName = resolveHostTagName(tagName).toLowerCase(); +function getSecurityContext( + tagName: string, + propName: string, + elementNamespace?: string | null, +): SecurityContext { + const resolvedElement = resolveElement(tagName, elementNamespace); + tagName = resolvedElement.tagName.toLowerCase(); propName = propName.toLowerCase(); + const namespace = normalizeElementNamespace(resolvedElement.namespace); - if (hasSecurityContext(RESOURCE_MAP, tagName, propName)) { + if (namespace) { + const namespaceContext = getSecurityContextForNamespace(tagName, propName, namespace); + if (namespaceContext !== undefined) { + return namespaceContext; + } + } + + return getSecurityContextForNamespace(tagName, propName, NO_NAMESPACE) ?? SecurityContext.NONE; +} + +function getSecurityContextForNamespace( + tagName: string, + propName: string, + namespace: string, +): SecurityContext | undefined { + if (hasSecurityContext(RESOURCE_MAP[namespace], tagName, propName)) { return SecurityContext.RESOURCE_URL; } - if (hasSecurityContext(URL_MAP, tagName, propName)) { + if (hasSecurityContext(URL_MAP[namespace], tagName, propName)) { return SecurityContext.URL; } - if (hasSecurityContext(HTML_MAP, tagName, propName)) { + if (hasSecurityContext(HTML_MAP[namespace], tagName, propName)) { return SecurityContext.HTML; } - return SecurityContext.NONE; + return undefined; } function hasSecurityContext( - map: Record | undefined>, + map: SecurityContextMap | undefined, tagName: string, propName: string, ): boolean { - return map[tagName]?.[propName] === true || map['*']?.[propName] === true; + return map?.[tagName]?.[propName] === true || map?.[MATCH_ALL_ELEMENTS]?.[propName] === true; } -function resolveHostTagName(tagName: string): string { - if (tagName !== TNodeName.DynamicHost) { - return tagName; - } - +function resolveElement( + tagName: string, + elementNamespace?: string | null, +): {tagName: string; namespace: string | null | undefined} { + let namespace = elementNamespace; const index = getSelectedIndex(); const tNode = index === -1 ? null : getSelectedTNode(); - if (tNode !== null && tNode.type & TNodeType.Element) { + + if (namespace === undefined) { + namespace = tNode?.namespace; + } + + if (tagName === TNodeName.DynamicHost && tNode !== null && tNode.type & TNodeType.Element) { const element = getNativeByTNode(tNode, getLView()) as RElement; if (element.tagName) { - return element.tagName.toLowerCase(); + tagName = element.tagName.toLowerCase(); + } + if (namespace == null) { + namespace = (element as RElement & {namespaceURI?: string | null}).namespaceURI; } } - return tagName; + return {tagName, namespace}; +} + +function normalizeElementNamespace(namespace: string | null | undefined): string | null { + if (!namespace) { + return null; + } + + const lowerNamespace = namespace.toLowerCase(); + switch (lowerNamespace) { + case SVG_NAMESPACE: + case SVG_NAMESPACE_URI: + return SVG_NAMESPACE; + case MATH_ML_NAMESPACE: + case MATH_ML_NAMESPACE_URI: + return MATH_ML_NAMESPACE; + default: + return null; + } } export function validateAgainstEventProperties(name: string) { diff --git a/packages/core/test/acceptance/security_spec.ts b/packages/core/test/acceptance/security_spec.ts index 0651a362d56b..706d8b447b51 100644 --- a/packages/core/test/acceptance/security_spec.ts +++ b/packages/core/test/acceptance/security_spec.ts @@ -29,6 +29,9 @@ import {RuntimeErrorCode} from '../../src/errors'; import {global} from '../../src/util/global'; import {ComponentFixture, TestBed} from '../../testing'; +const SVG_NAMESPACE_URI = 'http://www.w3.org/2000/svg'; +const MATH_ML_NAMESPACE_URI = 'http://www.w3.org/1998/Math/MathML'; + describe('comment node text escaping', () => { // see: https://html.spec.whatwg.org/multipage/syntax.html#comments [ @@ -898,6 +901,40 @@ describe('host binding sanitization', () => { url = hostBindingValue; } + @Directive({ + selector: '[href-carrier]', + host: {'[attr.href]': 'url'}, + }) + class HrefCarrierDirective { + url = hostBindingValue; + } + + @Directive({ + selector: '[xlink-href-carrier]', + host: {'[attr.xlink:href]': 'url'}, + }) + class XlinkHrefCarrierDirective { + url = hostBindingValue; + } + + @Component({ + template: ` + + + + + + `, + imports: [HrefCarrierDirective, XlinkHrefCarrierDirective], + }) + class SvgNamespaceHostBindingApp {} + + @Component({ + template: '', + imports: [HrefCarrierDirective], + }) + class MathNamespaceHostBindingApp {} + @Component({ selector: 'host-srcdoc-carrier', template: '', @@ -943,10 +980,14 @@ describe('host binding sanitization', () => { attrName: string, value: string, expected: string, + options: {namespace?: string; directive?: Type} = {}, ): Promise { hostBindingValue = value; - dynamicHostElement = document.createElement(tagName); - dynamicHostDirective = DataCarrierDirective; + dynamicHostElement = + options.namespace === undefined + ? document.createElement(tagName) + : document.createElementNS(options.namespace, tagName); + dynamicHostDirective = options.directive ?? DataCarrierDirective; const fixture = TestBed.createComponent(DynamicHostTestApp); try { @@ -973,6 +1014,21 @@ describe('host binding sanitization', () => { } } + async function expectTemplateHostAttribute( + type: Type, + selector: string, + attrName: string, + value: string, + expected: string, + ): Promise { + hostBindingValue = value; + const fixture = TestBed.createComponent(type); + await fixture.whenStable(); + + const element = fixture.nativeElement.querySelector(selector) as Element; + expect(element.getAttribute(attrName)).toBe(expected); + } + async function expectComponentHostAttribute( type: Type, tagName: string, @@ -1008,6 +1064,56 @@ describe('host binding sanitization', () => { ); }); + it('should sanitize href host bindings on SVG links', async () => { + await expectTemplateHostAttribute( + SvgNamespaceHostBindingApp, + '#svg-href', + 'href', + HOST_BINDING_UNSAFE_URL, + `unsafe:${HOST_BINDING_UNSAFE_URL}`, + ); + }); + + it('should not sanitize href host bindings on non-link SVG elements', async () => { + await expectTemplateHostAttribute( + SvgNamespaceHostBindingApp, + '#svg-rect', + 'href', + HOST_BINDING_UNSAFE_URL, + HOST_BINDING_UNSAFE_URL, + ); + }); + + it('should sanitize xlink:href host bindings on SVG links', async () => { + await expectTemplateHostAttribute( + SvgNamespaceHostBindingApp, + '#svg-xlink-href', + 'xlink:href', + HOST_BINDING_UNSAFE_URL, + `unsafe:${HOST_BINDING_UNSAFE_URL}`, + ); + }); + + it('should sanitize href host bindings on MathML elements', async () => { + await expectTemplateHostAttribute( + MathNamespaceHostBindingApp, + '#math-href', + 'href', + HOST_BINDING_UNSAFE_URL, + `unsafe:${HOST_BINDING_UNSAFE_URL}`, + ); + }); + + it('should sanitize href host bindings on dynamic MathML hosts as URLs', async () => { + await expectDynamicHostAttribute( + 'base', + 'href', + HOST_BINDING_UNSAFE_URL, + `unsafe:${HOST_BINDING_UNSAFE_URL}`, + {namespace: MATH_ML_NAMESPACE_URI, directive: HrefCarrierDirective}, + ); + }); + it('should sanitize a dynamic directive host binding against the concrete host element', async () => { @Component({ selector: 'iframe', @@ -1287,7 +1393,7 @@ describe('Component host element validation', () => { }) class MySvgSink {} - const svgScriptHost = document.createElementNS('http://www.w3.org/2000/svg', 'script'); + const svgScriptHost = document.createElementNS(SVG_NAMESPACE_URI, 'script'); document.head.appendChild(svgScriptHost); try { diff --git a/packages/core/test/bundling/animations-standalone/bundle.golden_symbols.json b/packages/core/test/bundling/animations-standalone/bundle.golden_symbols.json index e676e2c62d4b..a263250786bb 100644 --- a/packages/core/test/bundling/animations-standalone/bundle.golden_symbols.json +++ b/packages/core/test/bundling/animations-standalone/bundle.golden_symbols.json @@ -119,6 +119,7 @@ "HEADER_OFFSET", "HOST", "HOST_ATTR", + "HTML_MAP", "HYDRATION", "ID", "INJECTOR", @@ -137,7 +138,9 @@ "LEAVE_TOKEN_REGEX", "LOCALE_ID", "LOCALE_ID", + "MATCH_ALL_ELEMENTS", "MATH_ML_NAMESPACE", + "MATH_ML_NAMESPACE2", "MAXIMUM_REFRESH_RERUNS", "MAXIMUM_REFRESH_RERUNS", "MODIFIER_KEYS", @@ -168,6 +171,7 @@ "NOT_FOUND_CHECK_ONLY_ELEMENT_INJECTOR", "NOT_YET", "NO_CHANGE", + "NO_NAMESPACE", "NO_PARENT_INJECTOR", "NULL_INJECTOR", "NULL_REMOVAL_STATE", @@ -211,6 +215,7 @@ "REMOVE_STYLES_ON_COMPONENT_DESTROY", "REMOVE_STYLES_ON_COMPONENT_DESTROY_DEFAULT", "RENDERER", + "RESOURCE_MAP", "ROOT_SELECTOR", "RendererFactory2", "RendererStyleFlags2", @@ -230,6 +235,7 @@ "SUBSTITUTION_EXPR_END", "SUBSTITUTION_EXPR_START", "SVG_NAMESPACE", + "SVG_NAMESPACE2", "SafeSubscriber", "Sanitizer", "ShadowDomRenderer", @@ -255,6 +261,7 @@ "TracingService", "TransitionAnimationEngine", "TransitionAnimationPlayer", + "URL_MAP", "USE_VALUE", "UnsubscriptionError", "VIEW_REFS", diff --git a/packages/core/test/bundling/create_component/bundle.golden_symbols.json b/packages/core/test/bundling/create_component/bundle.golden_symbols.json index 6fa3a7cdde31..837c1dd03fce 100644 --- a/packages/core/test/bundling/create_component/bundle.golden_symbols.json +++ b/packages/core/test/bundling/create_component/bundle.golden_symbols.json @@ -84,6 +84,7 @@ "HEADER_OFFSET", "HOST", "HOST_ATTR", + "HTML_MAP", "HYDRATION", "HostComponent", "ID", @@ -101,7 +102,9 @@ "KeyEventsPlugin", "LOCALE_ID", "LOCALE_ID", + "MATCH_ALL_ELEMENTS", "MATH_ML_NAMESPACE", + "MATH_ML_NAMESPACE2", "MAXIMUM_REFRESH_RERUNS", "MAXIMUM_REFRESH_RERUNS", "MODIFIER_KEYS", @@ -128,6 +131,7 @@ "NOT_FOUND_CHECK_ONLY_ELEMENT_INJECTOR", "NOT_YET", "NO_CHANGE", + "NO_NAMESPACE", "NO_PARENT_INJECTOR", "NULL_INJECTOR", "NgModuleRef", @@ -162,6 +166,7 @@ "REMOVE_STYLES_ON_COMPONENT_DESTROY_DEFAULT", "RENDERER", "REQUIRED_UNSET_VALUE", + "RESOURCE_MAP", "RendererFactory2", "RendererStyleFlags2", "RetrievingInjector", @@ -174,6 +179,7 @@ "SIGNAL_NODE", "STABILITY_WARNING_THRESHOLD", "SVG_NAMESPACE", + "SVG_NAMESPACE2", "SafeSubscriber", "Sanitizer", "ShadowDomRenderer", @@ -191,6 +197,7 @@ "T_HOST", "TracingAction", "TracingService", + "URL_MAP", "USE_VALUE", "UnsubscriptionError", "VIEW_REFS", diff --git a/packages/core/test/bundling/defer/bundle.golden_symbols.json b/packages/core/test/bundling/defer/bundle.golden_symbols.json index 10972b74c699..f4f3ce0767b7 100644 --- a/packages/core/test/bundling/defer/bundle.golden_symbols.json +++ b/packages/core/test/bundling/defer/bundle.golden_symbols.json @@ -133,6 +133,7 @@ "FLAGS", "HEADER_OFFSET", "HOST", + "HTML_MAP", "HYDRATE_TRIGGER_CLEANUP_FNS", "HYDRATION", "ID", @@ -148,7 +149,9 @@ "LOADING_AFTER_SLOT", "LOCALE_ID", "LOCALE_ID", + "MATCH_ALL_ELEMENTS", "MATH_ML_NAMESPACE", + "MATH_ML_NAMESPACE2", "MAXIMUM_REFRESH_RERUNS", "MAXIMUM_REFRESH_RERUNS", "MINIMUM_SLOT", @@ -174,6 +177,7 @@ "NOT_FOUND_CHECK_ONLY_ELEMENT_INJECTOR", "NOT_YET", "NO_CHANGE", + "NO_NAMESPACE", "NO_PARENT_INJECTOR", "NULL_INJECTOR", "NgModuleRef", @@ -205,6 +209,7 @@ "REACTIVE_NODE", "REACTIVE_TEMPLATE_CONSUMER", "RENDERER", + "RESOURCE_MAP", "RendererFactory2", "RendererStyleFlags2", "RetrievingInjector", @@ -217,6 +222,7 @@ "SSR_UNIQUE_ID", "STABILITY_WARNING_THRESHOLD", "SVG_NAMESPACE", + "SVG_NAMESPACE2", "SafeSubscriber", "Sanitizer", "StandaloneService", @@ -233,6 +239,7 @@ "T_HOST", "TracingAction", "TracingService", + "URL_MAP", "USE_VALUE", "UnsubscriptionError", "VIEW_REFS", diff --git a/packages/core/test/bundling/forms_reactive/bundle.golden_symbols.json b/packages/core/test/bundling/forms_reactive/bundle.golden_symbols.json index ad2e5ba5d806..961cb8d498b6 100644 --- a/packages/core/test/bundling/forms_reactive/bundle.golden_symbols.json +++ b/packages/core/test/bundling/forms_reactive/bundle.golden_symbols.json @@ -125,6 +125,7 @@ "HEADER_OFFSET", "HOST", "HOST_ATTR", + "HTML_MAP", "HYDRATION", "ID", "INJECTOR", @@ -142,7 +143,9 @@ "KeyEventsPlugin", "LOCALE_ID", "LOCALE_ID", + "MATCH_ALL_ELEMENTS", "MATH_ML_NAMESPACE", + "MATH_ML_NAMESPACE2", "MAXIMUM_REFRESH_RERUNS", "MAXIMUM_REFRESH_RERUNS", "MODIFIER_KEYS", @@ -175,6 +178,7 @@ "NOT_FOUND_CHECK_ONLY_ELEMENT_INJECTOR", "NOT_YET", "NO_CHANGE", + "NO_NAMESPACE", "NO_PARENT_INJECTOR", "NULL_INJECTOR", "NgControl", @@ -221,6 +225,7 @@ "REMOVE_STYLES_ON_COMPONENT_DESTROY_DEFAULT", "RENDERER", "REQUIRED_VALIDATOR", + "RESOURCE_MAP", "ROOT_EFFECT_NODE", "ReactiveFormsComponent", "ReactiveFormsComponent_div_14_Template", @@ -241,6 +246,7 @@ "SIMPLE_CHANGES_STORE", "STABILITY_WARNING_THRESHOLD", "SVG_NAMESPACE", + "SVG_NAMESPACE2", "SafeSubscriber", "SafeValueImpl", "Sanitizer", @@ -268,6 +274,7 @@ "TracingAction", "TracingService", "UNSET", + "URL_MAP", "USE_PENDING_TASKS", "USE_VALUE", "UnsubscriptionError", diff --git a/packages/core/test/bundling/forms_template_driven/bundle.golden_symbols.json b/packages/core/test/bundling/forms_template_driven/bundle.golden_symbols.json index ddffa9d94a48..8dd2773c8a65 100644 --- a/packages/core/test/bundling/forms_template_driven/bundle.golden_symbols.json +++ b/packages/core/test/bundling/forms_template_driven/bundle.golden_symbols.json @@ -118,6 +118,7 @@ "HEADER_OFFSET", "HOST", "HOST_ATTR", + "HTML_MAP", "HYDRATION", "ID", "INJECTOR", @@ -135,7 +136,9 @@ "KeyEventsPlugin", "LOCALE_ID", "LOCALE_ID", + "MATCH_ALL_ELEMENTS", "MATH_ML_NAMESPACE", + "MATH_ML_NAMESPACE2", "MAXIMUM_REFRESH_RERUNS", "MAXIMUM_REFRESH_RERUNS", "MODIFIER_KEYS", @@ -167,6 +170,7 @@ "NOT_FOUND_CHECK_ONLY_ELEMENT_INJECTOR", "NOT_YET", "NO_CHANGE", + "NO_NAMESPACE", "NO_PARENT_INJECTOR", "NULL_INJECTOR", "NgControl", @@ -216,6 +220,7 @@ "REMOVE_STYLES_ON_COMPONENT_DESTROY_DEFAULT", "RENDERER", "REQUIRED_VALIDATOR", + "RESOURCE_MAP", "ROOT_EFFECT_NODE", "ReactiveValidationError", "Renderer2", @@ -233,6 +238,7 @@ "SIMPLE_CHANGES_STORE", "STABILITY_WARNING_THRESHOLD", "SVG_NAMESPACE", + "SVG_NAMESPACE2", "SafeSubscriber", "SafeValueImpl", "Sanitizer", @@ -262,6 +268,7 @@ "TracingAction", "TracingService", "UNSET", + "URL_MAP", "USE_PENDING_TASKS", "USE_VALUE", "UnsubscriptionError", diff --git a/packages/core/test/bundling/hydration/bundle.golden_symbols.json b/packages/core/test/bundling/hydration/bundle.golden_symbols.json index 8f1d3e8dabe1..3cf23c3b896d 100644 --- a/packages/core/test/bundling/hydration/bundle.golden_symbols.json +++ b/packages/core/test/bundling/hydration/bundle.golden_symbols.json @@ -127,6 +127,7 @@ "HEADER_OFFSET", "HOST", "HOST_ATTR", + "HTML_MAP", "HTTP_ROOT_INTERCEPTOR_FNS", "HTTP_TRANSFER_CACHE_ORIGIN_MAP", "HYDRATE_TRIGGER_CLEANUP_FNS", @@ -157,7 +158,9 @@ "LOADING_AFTER_SLOT", "LOCALE_ID", "LOCALE_ID", + "MATCH_ALL_ELEMENTS", "MATH_ML_NAMESPACE", + "MATH_ML_NAMESPACE2", "MAXIMUM_REFRESH_RERUNS", "MAXIMUM_REFRESH_RERUNS", "MINIMUM_SLOT", @@ -195,6 +198,7 @@ "NOT_FOUND_CHECK_ONLY_ELEMENT_INJECTOR", "NOT_YET", "NO_CHANGE", + "NO_NAMESPACE", "NO_PARENT_INJECTOR", "NULL_INJECTOR", "NUM_ROOT_NODES", @@ -239,6 +243,7 @@ "REMOVE_STYLES_ON_COMPONENT_DESTROY_DEFAULT", "RENDERER", "REQ_URL", + "RESOURCE_MAP", "RESPONSE_TYPE", "RendererFactory2", "RendererStyleFlags2", @@ -258,6 +263,7 @@ "STATUS", "STATUS_TEXT", "SVG_NAMESPACE", + "SVG_NAMESPACE2", "SafeSubscriber", "Sanitizer", "ShadowDomRenderer", @@ -283,6 +289,7 @@ "TracingService", "TransferState", "UNCACHEABLE_CACHE_CONTROL_DIRECTIVES", + "URL_MAP", "USE_VALUE", "UnsubscriptionError", "VIEW_REFS", diff --git a/packages/core/test/bundling/router/bundle.golden_symbols.json b/packages/core/test/bundling/router/bundle.golden_symbols.json index 0f8ef178137c..d16689637124 100644 --- a/packages/core/test/bundling/router/bundle.golden_symbols.json +++ b/packages/core/test/bundling/router/bundle.golden_symbols.json @@ -115,10 +115,10 @@ "FLAGS", "GuardsCheckEnd", "GuardsCheckStart", - "HTML_MAP", "HEADER_OFFSET", "HOST", "HOST_ATTR", + "HTML_MAP", "HYDRATION", "HistoryStateManager", "HostAttributeToken", @@ -147,7 +147,10 @@ "ListComponent", "Location", "LocationStrategy", + "MATCH_ALL_ELEMENTS", "MATH_ML_NAMESPACE", + "MATH_ML_NAMESPACE2", + "MATH_ML_NAMESPACE_URI", "MATRIX_PARAM_SEGMENT_RE", "MAXIMUM_REFRESH_RERUNS", "MAXIMUM_REFRESH_RERUNS", @@ -179,6 +182,7 @@ "NOT_FOUND_CHECK_ONLY_ELEMENT_INJECTOR", "NOT_YET", "NO_CHANGE", + "NO_NAMESPACE", "NO_PARENT_INJECTOR", "NULL_INJECTOR", "Navigation", @@ -281,6 +285,8 @@ "SIMPLE_CHANGES_STORE", "STABILITY_WARNING_THRESHOLD", "SVG_NAMESPACE", + "SVG_NAMESPACE2", + "SVG_NAMESPACE_URI", "SafeSubscriber", "SafeValueImpl", "Sanitizer", @@ -734,6 +740,7 @@ "getSanitizationBypassType", "getSanitizer", "getSecurityContext", + "getSecurityContextForNamespace", "getSelectedIndex", "getSelectedTNode", "getSimpleChangesStore", @@ -958,6 +965,7 @@ "noop2", "noop3", "normalizeBootstrapOptions", + "normalizeElementNamespace", "normalizeQueryParams", "normalizeQueryParams2", "notFoundValueOrThrow", @@ -1034,8 +1042,8 @@ "resetPreOrderHookFlags", "resolveData", "resolveDirectives", + "resolveElement", "resolveForwardRef", - "resolveHostTagName", "resolveNode", "retrieveHydrationInfo", "reusedNodes", @@ -1189,4 +1197,4 @@ ], "lazy": [] } -} +} \ No newline at end of file diff --git a/packages/core/test/bundling/standalone_bootstrap/bundle.golden_symbols.json b/packages/core/test/bundling/standalone_bootstrap/bundle.golden_symbols.json index 5455e4f8ae0b..26a1fdc732c1 100644 --- a/packages/core/test/bundling/standalone_bootstrap/bundle.golden_symbols.json +++ b/packages/core/test/bundling/standalone_bootstrap/bundle.golden_symbols.json @@ -81,6 +81,7 @@ "HEADER_OFFSET", "HOST", "HOST_ATTR", + "HTML_MAP", "HYDRATION", "HelloWorld", "ID", @@ -96,7 +97,9 @@ "KeyEventsPlugin", "LOCALE_ID", "LOCALE_ID", + "MATCH_ALL_ELEMENTS", "MATH_ML_NAMESPACE", + "MATH_ML_NAMESPACE2", "MAXIMUM_REFRESH_RERUNS", "MAXIMUM_REFRESH_RERUNS", "MODIFIER_KEYS", @@ -123,6 +126,7 @@ "NOT_FOUND_CHECK_ONLY_ELEMENT_INJECTOR", "NOT_YET", "NO_CHANGE", + "NO_NAMESPACE", "NO_PARENT_INJECTOR", "NULL_INJECTOR", "NgModuleRef", @@ -155,6 +159,7 @@ "REMOVE_STYLES_ON_COMPONENT_DESTROY", "REMOVE_STYLES_ON_COMPONENT_DESTROY_DEFAULT", "RENDERER", + "RESOURCE_MAP", "RendererFactory2", "RendererStyleFlags2", "RetrievingInjector", @@ -165,6 +170,7 @@ "SIGNAL", "STABILITY_WARNING_THRESHOLD", "SVG_NAMESPACE", + "SVG_NAMESPACE2", "SafeSubscriber", "Sanitizer", "ShadowDomRenderer", @@ -182,6 +188,7 @@ "T_HOST", "TracingAction", "TracingService", + "URL_MAP", "USE_VALUE", "UnsubscriptionError", "VIEW_REFS", diff --git a/packages/core/test/sanitization/sanitization_spec.ts b/packages/core/test/sanitization/sanitization_spec.ts index 04d8a9757e18..26822b9f2376 100644 --- a/packages/core/test/sanitization/sanitization_spec.ts +++ b/packages/core/test/sanitization/sanitization_spec.ts @@ -118,7 +118,6 @@ describe('sanitization', () => { // making sure security schema we have on compiler side is in sync with the `getUrlSanitizer` // runtime function definition const schema = SECURITY_SCHEMA(); - const contextsByProp: Map> = new Map(); const sanitizerNameByContext: Map = new Map([ [SecurityContext.URL, ɵɵsanitizeUrl], [SecurityContext.RESOURCE_URL, ɵɵsanitizeResourceUrl], @@ -126,30 +125,30 @@ describe('sanitization', () => { for (const [prop, nsSchema] of Object.entries(schema)) { for (const [ns, tagSchema] of Object.entries(nsSchema)) { - if (ns !== '') { - continue; - } - for (const [tag, context] of Object.entries(tagSchema)) { if (context !== SecurityContext.URL && context !== SecurityContext.RESOURCE_URL) { continue; } - const contexts = contextsByProp.get(prop) || new Set(); - contexts.add(context); - contextsByProp.set(prop, contexts); - - // check only in case a prop can be a part of both URL contexts - if (contexts.size === 2) { - expect(getUrlSanitizer(tag, prop)) - .withContext(`ns: ${ns}, tag: ${tag}, prop: ${prop}, context: ${context}`) - .toEqual(sanitizerNameByContext.get(context)!); - } + expect(getUrlSanitizer(tag, prop, ns)) + .withContext(`ns: ${ns}, tag: ${tag}, prop: ${prop}, context: ${context}`) + .toEqual(sanitizerNameByContext.get(context)!); } } } }); + it('should select URL sanitizers for namespaced URL props', () => { + expect(getUrlSanitizer('base', 'href', 'math')).toEqual(ɵɵsanitizeUrl); + expect(getUrlSanitizer('link', 'href', 'math')).toEqual(ɵɵsanitizeUrl); + expect(getUrlSanitizer('unknown', 'xlink:href', 'math')).toEqual(ɵɵsanitizeUrl); + + expect(getUrlSanitizer('a', 'href', 'svg')).toEqual(ɵɵsanitizeUrl); + expect(getUrlSanitizer('a', 'xlink:href', 'svg')).toEqual(ɵɵsanitizeUrl); + expect(getUrlSanitizer('rect', 'href', 'svg')).toBeNull(); + expect(getUrlSanitizer('rect', 'xlink:href', 'svg')).toBeNull(); + }); + it('should select URL sanitizer case-insensitively', () => { expect(getUrlSanitizer('IFRAME', 'SRC')).toEqual(ɵɵsanitizeResourceUrl); expect(getUrlSanitizer('IFRAME', 'src')).toEqual(ɵɵsanitizeResourceUrl); @@ -157,6 +156,8 @@ describe('sanitization', () => { expect(getUrlSanitizer('DiV', 'DaTa')).toBeNull(); expect(getUrlSanitizer('A', 'HREF')).toEqual(ɵɵsanitizeUrl); + expect(getUrlSanitizer('BASE', 'HREF', 'MATH')).toEqual(ɵɵsanitizeUrl); + expect(getUrlSanitizer('A', 'XLINK:HREF', 'SVG')).toEqual(ɵɵsanitizeUrl); }); it('should sanitize URL or ResourceURL case-insensitively', () => { From 35f895f9ba6ef68c586f4b8fc7e474632f0eb6d4 Mon Sep 17 00:00:00 2001 From: SkyZeroZx <73321943+SkyZeroZx@users.noreply.github.com> Date: Wed, 1 Jul 2026 11:44:00 -0500 Subject: [PATCH 3/7] fixup! fix(core): account for namespaces in host binding sanitization --- .../pipeline/src/phases/resolve_sanitizers.ts | 44 ++--- packages/core/src/render3/namespaces.ts | 2 + .../core/src/sanitization/sanitization.ts | 151 ++++-------------- .../core/test/acceptance/security_spec.ts | 32 ++++ .../bundle.golden_symbols.json | 7 - .../bundle.golden_symbols.json | 7 - .../bundling/defer/bundle.golden_symbols.json | 7 - .../forms_reactive/bundle.golden_symbols.json | 7 - .../bundle.golden_symbols.json | 7 - .../hydration/bundle.golden_symbols.json | 7 - .../router/bundle.golden_symbols.json | 12 +- .../bundle.golden_symbols.json | 7 - .../test/sanitization/sanitization_spec.ts | 22 +-- 13 files changed, 102 insertions(+), 210 deletions(-) diff --git a/packages/compiler/src/template/pipeline/src/phases/resolve_sanitizers.ts b/packages/compiler/src/template/pipeline/src/phases/resolve_sanitizers.ts index 1012a3beefae..d2c3731598ca 100644 --- a/packages/compiler/src/template/pipeline/src/phases/resolve_sanitizers.ts +++ b/packages/compiler/src/template/pipeline/src/phases/resolve_sanitizers.ts @@ -58,7 +58,10 @@ export function resolveSanitizers(job: CompilationJob): void { case ir.OpKind.DomProperty: case ir.OpKind.TwoWayProperty: let sanitizerFn: o.ExternalReference | null = null; - if (isUrlOrResourceUrlSecurityContext(op.securityContext)) { + if ( + Array.isArray(op.securityContext) && + hasCompositeUrlSecurityContext(op.securityContext) + ) { // When the host element isn't known, attributes such as `href`, `src`, `data`, // `action`, and `codebase` may be part of multiple security contexts. In this case we // use a special sanitization function and select the actual behavior at runtime based @@ -76,13 +79,7 @@ export function resolveSanitizers(job: CompilationJob): void { } } -function isUrlOrResourceUrlSecurityContext( - securityContext: SecurityContext | SecurityContext[], -): boolean { - if (!Array.isArray(securityContext)) { - return false; - } - +function hasCompositeUrlSecurityContext(securityContext: SecurityContext[]): boolean { let hasUrlContext = false; let hasResourceUrlContext = false; let hasNoneContext = false; @@ -115,18 +112,23 @@ function isUrlOrResourceUrlSecurityContext( function getOnlySecurityContext( securityContext: SecurityContext | SecurityContext[], ): SecurityContext { - if (Array.isArray(securityContext)) { - const nonNoneSecurityContexts = securityContext.filter( - (context) => context !== SecurityContext.NONE, - ); - if (nonNoneSecurityContexts.length > 1) { - // TODO: What should we do here? TDB just took the first one, but this feels like something we - // would want to know about and create a special case for like we did for Url/ResourceUrl. My - // guess is that, outside of the Url/ResourceUrl case, this never actually happens. If there - // do turn out to be other cases, throwing an error until we can address it feels safer. - throw Error(`AssertionError: Ambiguous security context`); - } - return nonNoneSecurityContexts[0] || SecurityContext.NONE; + if (!Array.isArray(securityContext)) { + return securityContext; + } + + if (securityContext.length < 2) { + return securityContext[0] ?? SecurityContext.NONE; + } + + const nonNoneSecurityContexts = securityContext.filter( + (context) => context !== SecurityContext.NONE, + ); + if (nonNoneSecurityContexts.length > 1) { + // TODO: What should we do here? TDB just took the first one, but this feels like something we + // would want to know about and create a special case for like we did for Url/ResourceUrl. My + // guess is that, outside of the Url/ResourceUrl case, this never actually happens. If there + // do turn out to be other cases, throwing an error until we can address it feels safer. + throw Error(`AssertionError: Ambiguous security context`); } - return securityContext; + return nonNoneSecurityContexts[0] ?? SecurityContext.NONE; } diff --git a/packages/core/src/render3/namespaces.ts b/packages/core/src/render3/namespaces.ts index 56ccf98f1029..b0997e24aa4b 100644 --- a/packages/core/src/render3/namespaces.ts +++ b/packages/core/src/render3/namespaces.ts @@ -8,3 +8,5 @@ export const SVG_NAMESPACE = 'svg'; export const MATH_ML_NAMESPACE = 'math'; +export const SVG_NAMESPACE_URI = 'http://www.w3.org/2000/svg'; +export const MATH_ML_NAMESPACE_URI = 'http://www.w3.org/1998/math/mathml'; diff --git a/packages/core/src/sanitization/sanitization.ts b/packages/core/src/sanitization/sanitization.ts index 7334807863e6..318d99e5f530 100644 --- a/packages/core/src/sanitization/sanitization.ts +++ b/packages/core/src/sanitization/sanitization.ts @@ -16,6 +16,12 @@ import {ENVIRONMENT} from '../render3/interfaces/view'; import {getLView, getSelectedIndex, getSelectedTNode} from '../render3/state'; import {renderStringify} from '../render3/util/stringify_utils'; import {getNativeByTNode} from '../render3/util/view_utils'; +import { + MATH_ML_NAMESPACE, + MATH_ML_NAMESPACE_URI, + SVG_NAMESPACE, + SVG_NAMESPACE_URI, +} from '../render3/namespaces'; import {TrustedHTML, TrustedScript, TrustedScriptURL} from '../util/security/trusted_type_defs'; import {trustedHTMLFromString, trustedScriptURLFromString} from '../util/security/trusted_types'; import { @@ -28,7 +34,7 @@ import {allowSanitizationBypassAndThrow, BypassType, unwrapSafeValue} from './by import {_sanitizeHtml} from './html_sanitizer'; import {enforceIframeSecurity} from './iframe_attrs_validation'; import {Sanitizer} from './sanitizer'; -import {SecurityContext} from './dom_security_schema'; +import {checkSecurityContext, SecurityContext} from './dom_security_schema'; import {_sanitizeUrl} from './url_sanitizer'; /** @@ -225,53 +231,6 @@ export function ɵɵtrustConstantResourceUrl(url: TemplateStringsArray): Trusted return trustedScriptURLFromString(url[0]); } -type SecurityContextMap = Record | undefined>; -type NamespacedSecurityContextMap = Record; - -const NO_NAMESPACE = ''; -const MATCH_ALL_ELEMENTS = '*'; -const SVG_NAMESPACE = 'svg'; -const MATH_ML_NAMESPACE = 'math'; -const SVG_NAMESPACE_URI = 'http://www.w3.org/2000/svg'; -const MATH_ML_NAMESPACE_URI = 'http://www.w3.org/1998/math/mathml'; - -const HTML_MAP: NamespacedSecurityContextMap = { - [NO_NAMESPACE]: { - [MATCH_ALL_ELEMENTS]: {'innerhtml': true, 'outerhtml': true}, - 'iframe': {'srcdoc': true}, - }, -}; - -const URL_MAP: NamespacedSecurityContextMap = { - [NO_NAMESPACE]: { - [MATCH_ALL_ELEMENTS]: {'formaction': true}, - 'area': {'href': true}, - 'a': {'href': true, 'xlink:href': true}, - 'form': {'action': true}, - 'img': {'src': true}, - 'video': {'src': true}, - }, - [MATH_ML_NAMESPACE]: { - [MATCH_ALL_ELEMENTS]: {'href': true, 'xlink:href': true}, - }, - [SVG_NAMESPACE]: { - 'a': {'href': true, 'xlink:href': true}, - }, -}; - -const RESOURCE_MAP: NamespacedSecurityContextMap = { - [NO_NAMESPACE]: { - 'embed': {'src': true}, - 'frame': {'src': true}, - 'iframe': {'src': true}, - 'media': {'src': true}, - - 'base': {'href': true}, - 'link': {'href': true}, - 'object': {'data': true, 'codebase': true}, - }, -}; - /** * Detects which sanitizer to use for URL property, based on tag name and prop name. * @@ -279,8 +238,8 @@ const RESOURCE_MAP: NamespacedSecurityContextMap = { * `packages/compiler/src/schema/dom_security_schema.ts`. * If tag and prop names don't match URL or Resource URL schema, no sanitizer is required. */ -export function getUrlSanitizer(tag: string, prop: string, elementNamespace?: string | null) { - switch (getSecurityContext(tag, prop, elementNamespace)) { +export function getUrlSanitizer(tag: string, prop: string) { + switch (getSecurityContext(tag, prop)) { case SecurityContext.RESOURCE_URL: return ɵɵsanitizeResourceUrl; case SecurityContext.URL: @@ -310,90 +269,35 @@ export function ɵɵsanitizeUrlOrResourceUrl(unsafeUrl: any, tag: string, prop: return sanitizer === null ? unsafeUrl : sanitizer(unsafeUrl); } -function getSecurityContext( - tagName: string, - propName: string, - elementNamespace?: string | null, -): SecurityContext { - const resolvedElement = resolveElement(tagName, elementNamespace); - tagName = resolvedElement.tagName.toLowerCase(); - propName = propName.toLowerCase(); - const namespace = normalizeElementNamespace(resolvedElement.namespace); - - if (namespace) { - const namespaceContext = getSecurityContextForNamespace(tagName, propName, namespace); - if (namespaceContext !== undefined) { - return namespaceContext; - } - } - - return getSecurityContextForNamespace(tagName, propName, NO_NAMESPACE) ?? SecurityContext.NONE; +function getSecurityContext(tagName: string, propName: string): SecurityContext { + const resolvedElement = resolveElement(tagName); + return checkSecurityContext(resolvedElement.tagName, propName, resolvedElement.namespace); } -function getSecurityContextForNamespace( - tagName: string, - propName: string, - namespace: string, -): SecurityContext | undefined { - if (hasSecurityContext(RESOURCE_MAP[namespace], tagName, propName)) { - return SecurityContext.RESOURCE_URL; - } - - if (hasSecurityContext(URL_MAP[namespace], tagName, propName)) { - return SecurityContext.URL; - } - - if (hasSecurityContext(HTML_MAP[namespace], tagName, propName)) { - return SecurityContext.HTML; - } - - return undefined; -} - -function hasSecurityContext( - map: SecurityContextMap | undefined, - tagName: string, - propName: string, -): boolean { - return map?.[tagName]?.[propName] === true || map?.[MATCH_ALL_ELEMENTS]?.[propName] === true; -} - -function resolveElement( - tagName: string, - elementNamespace?: string | null, -): {tagName: string; namespace: string | null | undefined} { - let namespace = elementNamespace; +function resolveElement(tagName: string): {tagName: string; namespace: string | null | undefined} { const index = getSelectedIndex(); const tNode = index === -1 ? null : getSelectedTNode(); - - if (namespace === undefined) { - namespace = tNode?.namespace; - } + let namespace = tNode?.namespace; if (tagName === TNodeName.DynamicHost && tNode !== null && tNode.type & TNodeType.Element) { const element = getNativeByTNode(tNode, getLView()) as RElement; if (element.tagName) { - tagName = element.tagName.toLowerCase(); + tagName = element.tagName; } if (namespace == null) { - namespace = (element as RElement & {namespaceURI?: string | null}).namespaceURI; + namespace = namespaceUriToKey( + (element as RElement & {namespaceURI?: string | null}).namespaceURI, + ); } } - return {tagName, namespace}; + return {tagName: tagName.toLowerCase(), namespace}; } -function normalizeElementNamespace(namespace: string | null | undefined): string | null { - if (!namespace) { - return null; - } - - const lowerNamespace = namespace.toLowerCase(); - switch (lowerNamespace) { - case SVG_NAMESPACE: +function namespaceUriToKey(namespaceUri: string | null | undefined): string | null { + switch (namespaceUri?.toLowerCase()) { case SVG_NAMESPACE_URI: return SVG_NAMESPACE; - case MATH_ML_NAMESPACE: case MATH_ML_NAMESPACE_URI: return MATH_ML_NAMESPACE; default: @@ -464,8 +368,15 @@ export function ɵɵvalidateAttribute(value: T, tagName: string, attrib return value; } + let namespace = tNode?.namespace; if (tagName === TNodeName.DynamicHost && tNode !== null) { - tagName = ((getNativeByTNode(tNode, getLView()) as RElement).tagName || tagName).toLowerCase(); + const element = getNativeByTNode(tNode, getLView()) as RElement; + tagName = (element.tagName || tagName).toLowerCase(); + if (namespace == null) { + namespace = namespaceUriToKey( + (element as RElement & {namespaceURI?: string | null}).namespaceURI, + ); + } } const lowerCaseTagName = tagName.toLowerCase(); @@ -473,8 +384,8 @@ export function ɵɵvalidateAttribute(value: T, tagName: string, attrib // Leverage tNode.namespace if active, otherwise check both namespaced and base variants. const fullTagName = - lowerCaseTagName[0] !== ':' && tNode?.namespace - ? `:${tNode.namespace}:${lowerCaseTagName}` + lowerCaseTagName[0] !== ':' && namespace + ? `:${namespace}:${lowerCaseTagName}` : lowerCaseTagName; const validationConfig = SECURITY_SENSITIVE_ELEMENTS[fullTagName]?.[lowerCaseAttrName]; diff --git a/packages/core/test/acceptance/security_spec.ts b/packages/core/test/acceptance/security_spec.ts index 706d8b447b51..385bc5cce671 100644 --- a/packages/core/test/acceptance/security_spec.ts +++ b/packages/core/test/acceptance/security_spec.ts @@ -1104,6 +1104,16 @@ describe('host binding sanitization', () => { ); }); + it('should sanitize href host bindings on dynamic SVG hosts as URLs', async () => { + await expectDynamicHostAttribute( + 'a', + 'href', + HOST_BINDING_UNSAFE_URL, + `unsafe:${HOST_BINDING_UNSAFE_URL}`, + {namespace: SVG_NAMESPACE_URI, directive: HrefCarrierDirective}, + ); + }); + it('should sanitize href host bindings on dynamic MathML hosts as URLs', async () => { await expectDynamicHostAttribute( 'base', @@ -1324,6 +1334,28 @@ describe('host binding sanitization', () => { } }); + it('should reject security-sensitive attribute host bindings on concrete dynamic SVG animation hosts', async () => { + @Directive({ + selector: 'attribute-name-carrier', + host: {'[attr.attributeName]': 'attributeName'}, + }) + class AttributeNameCarrierDirective { + attributeName = 'href'; + } + + dynamicHostElement = document.createElementNS(SVG_NAMESPACE_URI, 'animate'); + dynamicHostDirective = AttributeNameCarrierDirective; + const fixture = TestBed.createComponent(DynamicHostTestApp); + + try { + await expectAsync(fixture.whenStable()).toBeRejectedWithError( + /NG0910: Angular has detected that the `attributeName` was applied as a binding to the /, + ); + } finally { + fixture.componentInstance.componentRef.destroy(); + } + }); + it('should sanitize pure :not selector host bindings against a concrete hostElement', async () => { @Component({ selector: ':not(iframe)', diff --git a/packages/core/test/bundling/animations-standalone/bundle.golden_symbols.json b/packages/core/test/bundling/animations-standalone/bundle.golden_symbols.json index a263250786bb..e676e2c62d4b 100644 --- a/packages/core/test/bundling/animations-standalone/bundle.golden_symbols.json +++ b/packages/core/test/bundling/animations-standalone/bundle.golden_symbols.json @@ -119,7 +119,6 @@ "HEADER_OFFSET", "HOST", "HOST_ATTR", - "HTML_MAP", "HYDRATION", "ID", "INJECTOR", @@ -138,9 +137,7 @@ "LEAVE_TOKEN_REGEX", "LOCALE_ID", "LOCALE_ID", - "MATCH_ALL_ELEMENTS", "MATH_ML_NAMESPACE", - "MATH_ML_NAMESPACE2", "MAXIMUM_REFRESH_RERUNS", "MAXIMUM_REFRESH_RERUNS", "MODIFIER_KEYS", @@ -171,7 +168,6 @@ "NOT_FOUND_CHECK_ONLY_ELEMENT_INJECTOR", "NOT_YET", "NO_CHANGE", - "NO_NAMESPACE", "NO_PARENT_INJECTOR", "NULL_INJECTOR", "NULL_REMOVAL_STATE", @@ -215,7 +211,6 @@ "REMOVE_STYLES_ON_COMPONENT_DESTROY", "REMOVE_STYLES_ON_COMPONENT_DESTROY_DEFAULT", "RENDERER", - "RESOURCE_MAP", "ROOT_SELECTOR", "RendererFactory2", "RendererStyleFlags2", @@ -235,7 +230,6 @@ "SUBSTITUTION_EXPR_END", "SUBSTITUTION_EXPR_START", "SVG_NAMESPACE", - "SVG_NAMESPACE2", "SafeSubscriber", "Sanitizer", "ShadowDomRenderer", @@ -261,7 +255,6 @@ "TracingService", "TransitionAnimationEngine", "TransitionAnimationPlayer", - "URL_MAP", "USE_VALUE", "UnsubscriptionError", "VIEW_REFS", diff --git a/packages/core/test/bundling/create_component/bundle.golden_symbols.json b/packages/core/test/bundling/create_component/bundle.golden_symbols.json index 837c1dd03fce..6fa3a7cdde31 100644 --- a/packages/core/test/bundling/create_component/bundle.golden_symbols.json +++ b/packages/core/test/bundling/create_component/bundle.golden_symbols.json @@ -84,7 +84,6 @@ "HEADER_OFFSET", "HOST", "HOST_ATTR", - "HTML_MAP", "HYDRATION", "HostComponent", "ID", @@ -102,9 +101,7 @@ "KeyEventsPlugin", "LOCALE_ID", "LOCALE_ID", - "MATCH_ALL_ELEMENTS", "MATH_ML_NAMESPACE", - "MATH_ML_NAMESPACE2", "MAXIMUM_REFRESH_RERUNS", "MAXIMUM_REFRESH_RERUNS", "MODIFIER_KEYS", @@ -131,7 +128,6 @@ "NOT_FOUND_CHECK_ONLY_ELEMENT_INJECTOR", "NOT_YET", "NO_CHANGE", - "NO_NAMESPACE", "NO_PARENT_INJECTOR", "NULL_INJECTOR", "NgModuleRef", @@ -166,7 +162,6 @@ "REMOVE_STYLES_ON_COMPONENT_DESTROY_DEFAULT", "RENDERER", "REQUIRED_UNSET_VALUE", - "RESOURCE_MAP", "RendererFactory2", "RendererStyleFlags2", "RetrievingInjector", @@ -179,7 +174,6 @@ "SIGNAL_NODE", "STABILITY_WARNING_THRESHOLD", "SVG_NAMESPACE", - "SVG_NAMESPACE2", "SafeSubscriber", "Sanitizer", "ShadowDomRenderer", @@ -197,7 +191,6 @@ "T_HOST", "TracingAction", "TracingService", - "URL_MAP", "USE_VALUE", "UnsubscriptionError", "VIEW_REFS", diff --git a/packages/core/test/bundling/defer/bundle.golden_symbols.json b/packages/core/test/bundling/defer/bundle.golden_symbols.json index f4f3ce0767b7..10972b74c699 100644 --- a/packages/core/test/bundling/defer/bundle.golden_symbols.json +++ b/packages/core/test/bundling/defer/bundle.golden_symbols.json @@ -133,7 +133,6 @@ "FLAGS", "HEADER_OFFSET", "HOST", - "HTML_MAP", "HYDRATE_TRIGGER_CLEANUP_FNS", "HYDRATION", "ID", @@ -149,9 +148,7 @@ "LOADING_AFTER_SLOT", "LOCALE_ID", "LOCALE_ID", - "MATCH_ALL_ELEMENTS", "MATH_ML_NAMESPACE", - "MATH_ML_NAMESPACE2", "MAXIMUM_REFRESH_RERUNS", "MAXIMUM_REFRESH_RERUNS", "MINIMUM_SLOT", @@ -177,7 +174,6 @@ "NOT_FOUND_CHECK_ONLY_ELEMENT_INJECTOR", "NOT_YET", "NO_CHANGE", - "NO_NAMESPACE", "NO_PARENT_INJECTOR", "NULL_INJECTOR", "NgModuleRef", @@ -209,7 +205,6 @@ "REACTIVE_NODE", "REACTIVE_TEMPLATE_CONSUMER", "RENDERER", - "RESOURCE_MAP", "RendererFactory2", "RendererStyleFlags2", "RetrievingInjector", @@ -222,7 +217,6 @@ "SSR_UNIQUE_ID", "STABILITY_WARNING_THRESHOLD", "SVG_NAMESPACE", - "SVG_NAMESPACE2", "SafeSubscriber", "Sanitizer", "StandaloneService", @@ -239,7 +233,6 @@ "T_HOST", "TracingAction", "TracingService", - "URL_MAP", "USE_VALUE", "UnsubscriptionError", "VIEW_REFS", diff --git a/packages/core/test/bundling/forms_reactive/bundle.golden_symbols.json b/packages/core/test/bundling/forms_reactive/bundle.golden_symbols.json index 961cb8d498b6..ad2e5ba5d806 100644 --- a/packages/core/test/bundling/forms_reactive/bundle.golden_symbols.json +++ b/packages/core/test/bundling/forms_reactive/bundle.golden_symbols.json @@ -125,7 +125,6 @@ "HEADER_OFFSET", "HOST", "HOST_ATTR", - "HTML_MAP", "HYDRATION", "ID", "INJECTOR", @@ -143,9 +142,7 @@ "KeyEventsPlugin", "LOCALE_ID", "LOCALE_ID", - "MATCH_ALL_ELEMENTS", "MATH_ML_NAMESPACE", - "MATH_ML_NAMESPACE2", "MAXIMUM_REFRESH_RERUNS", "MAXIMUM_REFRESH_RERUNS", "MODIFIER_KEYS", @@ -178,7 +175,6 @@ "NOT_FOUND_CHECK_ONLY_ELEMENT_INJECTOR", "NOT_YET", "NO_CHANGE", - "NO_NAMESPACE", "NO_PARENT_INJECTOR", "NULL_INJECTOR", "NgControl", @@ -225,7 +221,6 @@ "REMOVE_STYLES_ON_COMPONENT_DESTROY_DEFAULT", "RENDERER", "REQUIRED_VALIDATOR", - "RESOURCE_MAP", "ROOT_EFFECT_NODE", "ReactiveFormsComponent", "ReactiveFormsComponent_div_14_Template", @@ -246,7 +241,6 @@ "SIMPLE_CHANGES_STORE", "STABILITY_WARNING_THRESHOLD", "SVG_NAMESPACE", - "SVG_NAMESPACE2", "SafeSubscriber", "SafeValueImpl", "Sanitizer", @@ -274,7 +268,6 @@ "TracingAction", "TracingService", "UNSET", - "URL_MAP", "USE_PENDING_TASKS", "USE_VALUE", "UnsubscriptionError", diff --git a/packages/core/test/bundling/forms_template_driven/bundle.golden_symbols.json b/packages/core/test/bundling/forms_template_driven/bundle.golden_symbols.json index 8dd2773c8a65..ddffa9d94a48 100644 --- a/packages/core/test/bundling/forms_template_driven/bundle.golden_symbols.json +++ b/packages/core/test/bundling/forms_template_driven/bundle.golden_symbols.json @@ -118,7 +118,6 @@ "HEADER_OFFSET", "HOST", "HOST_ATTR", - "HTML_MAP", "HYDRATION", "ID", "INJECTOR", @@ -136,9 +135,7 @@ "KeyEventsPlugin", "LOCALE_ID", "LOCALE_ID", - "MATCH_ALL_ELEMENTS", "MATH_ML_NAMESPACE", - "MATH_ML_NAMESPACE2", "MAXIMUM_REFRESH_RERUNS", "MAXIMUM_REFRESH_RERUNS", "MODIFIER_KEYS", @@ -170,7 +167,6 @@ "NOT_FOUND_CHECK_ONLY_ELEMENT_INJECTOR", "NOT_YET", "NO_CHANGE", - "NO_NAMESPACE", "NO_PARENT_INJECTOR", "NULL_INJECTOR", "NgControl", @@ -220,7 +216,6 @@ "REMOVE_STYLES_ON_COMPONENT_DESTROY_DEFAULT", "RENDERER", "REQUIRED_VALIDATOR", - "RESOURCE_MAP", "ROOT_EFFECT_NODE", "ReactiveValidationError", "Renderer2", @@ -238,7 +233,6 @@ "SIMPLE_CHANGES_STORE", "STABILITY_WARNING_THRESHOLD", "SVG_NAMESPACE", - "SVG_NAMESPACE2", "SafeSubscriber", "SafeValueImpl", "Sanitizer", @@ -268,7 +262,6 @@ "TracingAction", "TracingService", "UNSET", - "URL_MAP", "USE_PENDING_TASKS", "USE_VALUE", "UnsubscriptionError", diff --git a/packages/core/test/bundling/hydration/bundle.golden_symbols.json b/packages/core/test/bundling/hydration/bundle.golden_symbols.json index 3cf23c3b896d..8f1d3e8dabe1 100644 --- a/packages/core/test/bundling/hydration/bundle.golden_symbols.json +++ b/packages/core/test/bundling/hydration/bundle.golden_symbols.json @@ -127,7 +127,6 @@ "HEADER_OFFSET", "HOST", "HOST_ATTR", - "HTML_MAP", "HTTP_ROOT_INTERCEPTOR_FNS", "HTTP_TRANSFER_CACHE_ORIGIN_MAP", "HYDRATE_TRIGGER_CLEANUP_FNS", @@ -158,9 +157,7 @@ "LOADING_AFTER_SLOT", "LOCALE_ID", "LOCALE_ID", - "MATCH_ALL_ELEMENTS", "MATH_ML_NAMESPACE", - "MATH_ML_NAMESPACE2", "MAXIMUM_REFRESH_RERUNS", "MAXIMUM_REFRESH_RERUNS", "MINIMUM_SLOT", @@ -198,7 +195,6 @@ "NOT_FOUND_CHECK_ONLY_ELEMENT_INJECTOR", "NOT_YET", "NO_CHANGE", - "NO_NAMESPACE", "NO_PARENT_INJECTOR", "NULL_INJECTOR", "NUM_ROOT_NODES", @@ -243,7 +239,6 @@ "REMOVE_STYLES_ON_COMPONENT_DESTROY_DEFAULT", "RENDERER", "REQ_URL", - "RESOURCE_MAP", "RESPONSE_TYPE", "RendererFactory2", "RendererStyleFlags2", @@ -263,7 +258,6 @@ "STATUS", "STATUS_TEXT", "SVG_NAMESPACE", - "SVG_NAMESPACE2", "SafeSubscriber", "Sanitizer", "ShadowDomRenderer", @@ -289,7 +283,6 @@ "TracingService", "TransferState", "UNCACHEABLE_CACHE_CONTROL_DIRECTIVES", - "URL_MAP", "USE_VALUE", "UnsubscriptionError", "VIEW_REFS", diff --git a/packages/core/test/bundling/router/bundle.golden_symbols.json b/packages/core/test/bundling/router/bundle.golden_symbols.json index d16689637124..8c5f40a12ea3 100644 --- a/packages/core/test/bundling/router/bundle.golden_symbols.json +++ b/packages/core/test/bundling/router/bundle.golden_symbols.json @@ -118,7 +118,6 @@ "HEADER_OFFSET", "HOST", "HOST_ATTR", - "HTML_MAP", "HYDRATION", "HistoryStateManager", "HostAttributeToken", @@ -242,7 +241,6 @@ "REMOVE_STYLES_ON_COMPONENT_DESTROY_DEFAULT", "RENDERER", "REQUIRED_UNSET_VALUE", - "RESOURCE_MAP", "ROUTER_CONFIGURATION", "ROUTER_OUTLET_DATA", "ROUTER_PRELOADER", @@ -278,6 +276,7 @@ "SAFE_URL_PATTERN", "SCHEDULE_IN_ROOT_ZONE", "SCHEDULE_IN_ROOT_ZONE_DEFAULT", + "SECURITY_SCHEMA", "SEGMENT_RE", "SHARED_STYLES_HOST", "SIGNAL", @@ -317,7 +316,6 @@ "Tree", "TreeNode", "UNSET", - "URL_MAP", "USE_VALUE", "UnsubscriptionError", "UrlHandlingStrategy", @@ -359,6 +357,7 @@ "\\u0275\\u0275text", "\\u0275\\u0275textInterpolate1", "_DOM", + "_SECURITY_SCHEMA", "_THROW_IF_NOT_FOUND", "__asyncGenerator", "__asyncValues", @@ -455,6 +454,7 @@ "captureError", "catchError", "checkGuards", + "checkSecurityContext", "checkStable", "classIndexOf", "cleanUpView", @@ -524,6 +524,7 @@ "createNode", "createNodeInjector", "createNotification", + "createNullObj", "createObject", "createOperatorSubscriber", "createOrReuseChildren", @@ -740,7 +741,6 @@ "getSanitizationBypassType", "getSanitizer", "getSecurityContext", - "getSecurityContextForNamespace", "getSelectedIndex", "getSelectedTNode", "getSimpleChangesStore", @@ -767,7 +767,6 @@ "hasLift", "hasOnDestroy", "hasParentInjector", - "hasSecurityContext", "hasStaticTitle", "hasStyleInput", "hasTagAndTypeMatch", @@ -947,6 +946,7 @@ "mergeTrivialChildren", "moduleBootstrapImpl", "namedOutletsRedirect", + "namespaceUriToKey", "nativeAppendChild", "nativeAppendOrInsertBefore", "nativeInsertBefore", @@ -965,7 +965,6 @@ "noop2", "noop3", "normalizeBootstrapOptions", - "normalizeElementNamespace", "normalizeQueryParams", "normalizeQueryParams2", "notFoundValueOrThrow", @@ -1019,6 +1018,7 @@ "redirectingNavigationError", "refreshContentQueries", "refreshView", + "registerContext", "registerHostBindingOpCodes", "registerLView", "registerPostOrderHooks", diff --git a/packages/core/test/bundling/standalone_bootstrap/bundle.golden_symbols.json b/packages/core/test/bundling/standalone_bootstrap/bundle.golden_symbols.json index 26a1fdc732c1..5455e4f8ae0b 100644 --- a/packages/core/test/bundling/standalone_bootstrap/bundle.golden_symbols.json +++ b/packages/core/test/bundling/standalone_bootstrap/bundle.golden_symbols.json @@ -81,7 +81,6 @@ "HEADER_OFFSET", "HOST", "HOST_ATTR", - "HTML_MAP", "HYDRATION", "HelloWorld", "ID", @@ -97,9 +96,7 @@ "KeyEventsPlugin", "LOCALE_ID", "LOCALE_ID", - "MATCH_ALL_ELEMENTS", "MATH_ML_NAMESPACE", - "MATH_ML_NAMESPACE2", "MAXIMUM_REFRESH_RERUNS", "MAXIMUM_REFRESH_RERUNS", "MODIFIER_KEYS", @@ -126,7 +123,6 @@ "NOT_FOUND_CHECK_ONLY_ELEMENT_INJECTOR", "NOT_YET", "NO_CHANGE", - "NO_NAMESPACE", "NO_PARENT_INJECTOR", "NULL_INJECTOR", "NgModuleRef", @@ -159,7 +155,6 @@ "REMOVE_STYLES_ON_COMPONENT_DESTROY", "REMOVE_STYLES_ON_COMPONENT_DESTROY_DEFAULT", "RENDERER", - "RESOURCE_MAP", "RendererFactory2", "RendererStyleFlags2", "RetrievingInjector", @@ -170,7 +165,6 @@ "SIGNAL", "STABILITY_WARNING_THRESHOLD", "SVG_NAMESPACE", - "SVG_NAMESPACE2", "SafeSubscriber", "Sanitizer", "ShadowDomRenderer", @@ -188,7 +182,6 @@ "T_HOST", "TracingAction", "TracingService", - "URL_MAP", "USE_VALUE", "UnsubscriptionError", "VIEW_REFS", diff --git a/packages/core/test/sanitization/sanitization_spec.ts b/packages/core/test/sanitization/sanitization_spec.ts index 26822b9f2376..c8d794fe4d87 100644 --- a/packages/core/test/sanitization/sanitization_spec.ts +++ b/packages/core/test/sanitization/sanitization_spec.ts @@ -125,12 +125,19 @@ describe('sanitization', () => { for (const [prop, nsSchema] of Object.entries(schema)) { for (const [ns, tagSchema] of Object.entries(nsSchema)) { + // `getUrlSanitizer` resolves namespaces from the selected runtime `TNode`, so direct + // unit tests only cover non-namespaced schema entries. Namespaced host bindings are + // covered by acceptance tests. + if (ns !== '') { + continue; + } + for (const [tag, context] of Object.entries(tagSchema)) { if (context !== SecurityContext.URL && context !== SecurityContext.RESOURCE_URL) { continue; } - expect(getUrlSanitizer(tag, prop, ns)) + expect(getUrlSanitizer(tag, prop)) .withContext(`ns: ${ns}, tag: ${tag}, prop: ${prop}, context: ${context}`) .toEqual(sanitizerNameByContext.get(context)!); } @@ -138,17 +145,6 @@ describe('sanitization', () => { } }); - it('should select URL sanitizers for namespaced URL props', () => { - expect(getUrlSanitizer('base', 'href', 'math')).toEqual(ɵɵsanitizeUrl); - expect(getUrlSanitizer('link', 'href', 'math')).toEqual(ɵɵsanitizeUrl); - expect(getUrlSanitizer('unknown', 'xlink:href', 'math')).toEqual(ɵɵsanitizeUrl); - - expect(getUrlSanitizer('a', 'href', 'svg')).toEqual(ɵɵsanitizeUrl); - expect(getUrlSanitizer('a', 'xlink:href', 'svg')).toEqual(ɵɵsanitizeUrl); - expect(getUrlSanitizer('rect', 'href', 'svg')).toBeNull(); - expect(getUrlSanitizer('rect', 'xlink:href', 'svg')).toBeNull(); - }); - it('should select URL sanitizer case-insensitively', () => { expect(getUrlSanitizer('IFRAME', 'SRC')).toEqual(ɵɵsanitizeResourceUrl); expect(getUrlSanitizer('IFRAME', 'src')).toEqual(ɵɵsanitizeResourceUrl); @@ -156,8 +152,6 @@ describe('sanitization', () => { expect(getUrlSanitizer('DiV', 'DaTa')).toBeNull(); expect(getUrlSanitizer('A', 'HREF')).toEqual(ɵɵsanitizeUrl); - expect(getUrlSanitizer('BASE', 'HREF', 'MATH')).toEqual(ɵɵsanitizeUrl); - expect(getUrlSanitizer('A', 'XLINK:HREF', 'SVG')).toEqual(ɵɵsanitizeUrl); }); it('should sanitize URL or ResourceURL case-insensitively', () => { From 3c721ba8530825f7b89c56c4582317eddcc5f237 Mon Sep 17 00:00:00 2001 From: Alan Agius <17563226+alan-agius4@users.noreply.github.com> Date: Thu, 2 Jul 2026 08:49:43 +0000 Subject: [PATCH 4/7] fixup! fix(core): account for namespaces in host binding sanitization --- packages/core/src/render3/i18n/i18n_parse.ts | 31 +-- packages/core/src/render3/namespaces.ts | 7 +- packages/core/src/render3/util/tags.ts | 32 +++ .../core/src/sanitization/sanitization.ts | 195 +++++++----------- .../bundle.golden_symbols.json | 2 +- .../bundle.golden_symbols.json | 2 +- .../forms_reactive/bundle.golden_symbols.json | 2 +- .../bundle.golden_symbols.json | 2 +- .../hydration/bundle.golden_symbols.json | 2 +- .../router/bundle.golden_symbols.json | 5 +- .../bundle.golden_symbols.json | 2 +- 11 files changed, 122 insertions(+), 160 deletions(-) create mode 100644 packages/core/src/render3/util/tags.ts diff --git a/packages/core/src/render3/i18n/i18n_parse.ts b/packages/core/src/render3/i18n/i18n_parse.ts index b2a9bae32d4c..ea7056ebbc12 100644 --- a/packages/core/src/render3/i18n/i18n_parse.ts +++ b/packages/core/src/render3/i18n/i18n_parse.ts @@ -73,6 +73,8 @@ import { setTIcu, setTNodeInsertBeforeIndex, } from './i18n_util'; +import {splitNsName} from '../util/tags'; +import {NAMESPACE_URIS} from '../namespaces'; const BINDING_REGEXP = /�(\d+):?\d*�/gi; const ICU_REGEXP = /({\s*�\d+:?\d*�\s*,\s*\S{6}\s*,[\s\S]*})/gi; @@ -814,13 +816,10 @@ function walkIcuTree( const attr = elAttrs.item(i)!; const lowerAttrName = attr.name.toLowerCase(); const hasBinding = !!attr.value.match(BINDING_REGEXP); - const elementNS = element.namespaceURI; - const tagNameWithNamespace = - elementNS === 'http://www.w3.org/2000/svg' - ? `:svg:${tagName}` - : elementNS === 'http://www.w3.org/1998/Math/MathML' - ? `:math:${tagName}` - : tagName; + const namespaceUri = element.namespaceURI; + const namespace = namespaceUri && NAMESPACE_URIS[namespaceUri]; + const tagNameWithNamespace = namespace ? `:${namespace}:${tagName}` : tagName; + if (hasBinding) { if (VALID_ATTRS.hasOwnProperty(lowerAttrName)) { generateBindingUpdateOpCodes( @@ -984,24 +983,6 @@ function addCreateAttribute( create.push((newIndex << IcuCreateOpCode.SHIFT_REF) | IcuCreateOpCode.Attr, attrName, attrValue); } -function splitNsName(elementName: string, fatal: boolean = true): [string | null, string] { - if (elementName[0] != ':') { - return [null, elementName]; - } - - const colonIndex = elementName.indexOf(':', 1); - - if (colonIndex === -1) { - if (fatal) { - throw new Error(`Unsupported format "${elementName}" expecting ":namespace:name"`); - } else { - return [null, elementName]; - } - } - - return [elementName.slice(1, colonIndex), elementName.slice(colonIndex + 1)]; -} - function i18nResolveSanitizer(attrName: string, tagName?: string): SanitizerFn | null { let schemaContext: SecurityContext; diff --git a/packages/core/src/render3/namespaces.ts b/packages/core/src/render3/namespaces.ts index b0997e24aa4b..8f40f9d80f62 100644 --- a/packages/core/src/render3/namespaces.ts +++ b/packages/core/src/render3/namespaces.ts @@ -8,5 +8,8 @@ export const SVG_NAMESPACE = 'svg'; export const MATH_ML_NAMESPACE = 'math'; -export const SVG_NAMESPACE_URI = 'http://www.w3.org/2000/svg'; -export const MATH_ML_NAMESPACE_URI = 'http://www.w3.org/1998/math/mathml'; + +export const NAMESPACE_URIS: Record = { + 'http://www.w3.org/2000/svg': SVG_NAMESPACE, + 'http://www.w3.org/1998/Math/MathML': MATH_ML_NAMESPACE, +}; diff --git a/packages/core/src/render3/util/tags.ts b/packages/core/src/render3/util/tags.ts new file mode 100644 index 000000000000..b9c57f5d0b23 --- /dev/null +++ b/packages/core/src/render3/util/tags.ts @@ -0,0 +1,32 @@ +/** + * @license + * Copyright Google LLC All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.dev/license + */ + +/** + * Splits an element name into its namespace and local name. + * + * @param elementName The element name to split, in the format ":namespace:name". + * @param fatal If true, throws an error if the element name is not in the correct format. + * @returns A tuple containing the namespace and local name. + */ +export function splitNsName(elementName: string, fatal: boolean = true): [string | null, string] { + if (elementName[0] != ':') { + return [null, elementName]; + } + + const colonIndex = elementName.indexOf(':', 1); + + if (colonIndex === -1) { + if (fatal) { + throw new Error(`Unsupported format "${elementName}" expecting ":namespace:name"`); + } else { + return [null, elementName]; + } + } + + return [elementName.slice(1, colonIndex), elementName.slice(colonIndex + 1)]; +} diff --git a/packages/core/src/sanitization/sanitization.ts b/packages/core/src/sanitization/sanitization.ts index 318d99e5f530..fa86befe3d3b 100644 --- a/packages/core/src/sanitization/sanitization.ts +++ b/packages/core/src/sanitization/sanitization.ts @@ -16,12 +16,7 @@ import {ENVIRONMENT} from '../render3/interfaces/view'; import {getLView, getSelectedIndex, getSelectedTNode} from '../render3/state'; import {renderStringify} from '../render3/util/stringify_utils'; import {getNativeByTNode} from '../render3/util/view_utils'; -import { - MATH_ML_NAMESPACE, - MATH_ML_NAMESPACE_URI, - SVG_NAMESPACE, - SVG_NAMESPACE_URI, -} from '../render3/namespaces'; +import {NAMESPACE_URIS, SVG_NAMESPACE} from '../render3/namespaces'; import {TrustedHTML, TrustedScript, TrustedScriptURL} from '../util/security/trusted_type_defs'; import {trustedHTMLFromString, trustedScriptURLFromString} from '../util/security/trusted_types'; import { @@ -36,6 +31,7 @@ import {enforceIframeSecurity} from './iframe_attrs_validation'; import {Sanitizer} from './sanitizer'; import {checkSecurityContext, SecurityContext} from './dom_security_schema'; import {_sanitizeUrl} from './url_sanitizer'; +import {splitNsName} from '../render3/util/tags'; /** * An `html` sanitizer which converts untrusted `html` **string** into trusted string by removing @@ -264,45 +260,8 @@ export function getUrlSanitizer(tag: string, prop: string) { * * @codeGenApi */ -export function ɵɵsanitizeUrlOrResourceUrl(unsafeUrl: any, tag: string, prop: string): any { - const sanitizer = getUrlSanitizer(tag, prop); - return sanitizer === null ? unsafeUrl : sanitizer(unsafeUrl); -} - -function getSecurityContext(tagName: string, propName: string): SecurityContext { - const resolvedElement = resolveElement(tagName); - return checkSecurityContext(resolvedElement.tagName, propName, resolvedElement.namespace); -} - -function resolveElement(tagName: string): {tagName: string; namespace: string | null | undefined} { - const index = getSelectedIndex(); - const tNode = index === -1 ? null : getSelectedTNode(); - let namespace = tNode?.namespace; - - if (tagName === TNodeName.DynamicHost && tNode !== null && tNode.type & TNodeType.Element) { - const element = getNativeByTNode(tNode, getLView()) as RElement; - if (element.tagName) { - tagName = element.tagName; - } - if (namespace == null) { - namespace = namespaceUriToKey( - (element as RElement & {namespaceURI?: string | null}).namespaceURI, - ); - } - } - - return {tagName: tagName.toLowerCase(), namespace}; -} - -function namespaceUriToKey(namespaceUri: string | null | undefined): string | null { - switch (namespaceUri?.toLowerCase()) { - case SVG_NAMESPACE_URI: - return SVG_NAMESPACE; - case MATH_ML_NAMESPACE_URI: - return MATH_ML_NAMESPACE; - default: - return null; - } +export function ɵɵsanitizeUrlOrResourceUrl(unsafeUrl: any, tag: string, prop: string) { + return getUrlSanitizer(tag, prop)?.(unsafeUrl) ?? unsafeUrl; } export function validateAgainstEventProperties(name: string) { @@ -321,6 +280,37 @@ function getSanitizer(): Sanitizer | null { return lView && lView[ENVIRONMENT].sanitizer; } +function getSecurityContext(tagName: string, propName: string): SecurityContext { + const [namespace, resolvedTagName] = resolveElement(tagName); + return checkSecurityContext(resolvedTagName, propName, namespace); +} + +function resolveElement(tagName: string): [namespace: string | null | undefined, tagName: string] { + tagName = tagName.toLowerCase(); + const splitResult = splitNsName(tagName, false); + if (splitResult[0]) { + return splitResult; + } + + const index = getSelectedIndex(); + const tNode = index === -1 ? null : getSelectedTNode(); + let namespace = tNode?.namespace; + + if (tagName === TNodeName.DynamicHost && tNode?.type === TNodeType.Element) { + const element = getNativeByTNode(tNode, getLView()) as RElement; + if (element.tagName) { + tagName = element.tagName.toLowerCase(); + } + + if (namespace == null) { + const namespaceURI = (element as RElement & {namespaceURI?: string | null}).namespaceURI; + namespace = namespaceURI && NAMESPACE_URIS[namespaceURI]; + } + } + + return [namespace, tagName]; +} + /** * Set of attributes that are sensitive and should be sanitized. */ @@ -330,28 +320,16 @@ const SECURITY_SENSITIVE_ATTRIBUTE_NAMES: ReadonlySet = new Set(['href', * @remarks Keep this in sync with DOM Security Schema. * @see [SECURITY_SCHEMA](../../../compiler/src/schema/dom_security_schema.ts) */ -const SECURITY_SENSITIVE_ELEMENTS: Record< +const SVG_ANIMATION_SENSITIVE_STATIC_VALUES: Record< string, - Record> | undefined + Record> | undefined > = { - 'iframe': { - 'sandbox': true, - 'allow': true, - 'allowfullscreen': true, - 'referrerpolicy': true, - 'csp': true, - 'fetchpriority': true, - 'credentialless': true, - }, - ':svg:animate': { - 'attributename': true, + 'animate': { 'to': SECURITY_SENSITIVE_ATTRIBUTE_NAMES, 'values': SECURITY_SENSITIVE_ATTRIBUTE_NAMES, 'from': SECURITY_SENSITIVE_ATTRIBUTE_NAMES, }, - ':svg:set': {'attributename': true, 'to': SECURITY_SENSITIVE_ATTRIBUTE_NAMES}, - ':svg:animatemotion': {'attributename': true}, - ':svg:animatetransform': {'attributename': true}, + 'set': {'to': SECURITY_SENSITIVE_ATTRIBUTE_NAMES}, }; /** @@ -368,83 +346,52 @@ export function ɵɵvalidateAttribute(value: T, tagName: string, attrib return value; } - let namespace = tNode?.namespace; - if (tagName === TNodeName.DynamicHost && tNode !== null) { - const element = getNativeByTNode(tNode, getLView()) as RElement; - tagName = (element.tagName || tagName).toLowerCase(); - if (namespace == null) { - namespace = namespaceUriToKey( - (element as RElement & {namespaceURI?: string | null}).namespaceURI, - ); - } - } - - const lowerCaseTagName = tagName.toLowerCase(); - const lowerCaseAttrName = attributeName.toLowerCase(); - - // Leverage tNode.namespace if active, otherwise check both namespaced and base variants. - const fullTagName = - lowerCaseTagName[0] !== ':' && namespace - ? `:${namespace}:${lowerCaseTagName}` - : lowerCaseTagName; + const [namespace, resolvedTagName] = resolveElement(tagName); + const securityContext = checkSecurityContext(resolvedTagName, attributeName, namespace); - const validationConfig = SECURITY_SENSITIVE_ELEMENTS[fullTagName]?.[lowerCaseAttrName]; - - if (!validationConfig) { + if (securityContext !== SecurityContext.ATTRIBUTE_NO_BINDING) { return value; } const lView = getLView(); - if (tNode && lowerCaseTagName === 'iframe') { - const element = getNativeByTNode(tNode, lView) as RElement; - enforceIframeSecurity(element as HTMLIFrameElement); - } - - const displayTagName = tagName[0] === ':' ? tagName.split(':').pop()! : tagName; - - if (typeof validationConfig !== 'boolean') { - if (!tNode) { - const errorMessage = - ngDevMode && - `Angular has detected that the \`${attributeName}\` was applied ` + - `as a binding to the <${tagName}> element. ` + - `For security reasons, the \`${attributeName}\` can be set on the <${tagName}> element ` + - `as a static attribute only. \n` + - `To fix this, switch the \`${attributeName}\` binding to a static attribute ` + - `in a template or in host bindings section.`; - throw new RuntimeError(RuntimeErrorCode.UNSAFE_ATTRIBUTE_BINDING, errorMessage); + if (tNode) { + if (resolvedTagName === 'iframe') { + const element = getNativeByTNode(tNode, lView) as RElement; + enforceIframeSecurity(element as HTMLIFrameElement); + } else if (namespace === SVG_NAMESPACE) { + const config = + SVG_ANIMATION_SENSITIVE_STATIC_VALUES[resolvedTagName]?.[attributeName.toLowerCase()]; + if (config) { + const element = getNativeByTNode(tNode, lView) as SVGAnimateElement; + const attributeNameValue = getSecuritySensitiveSVGAnimationAttributeName(element, config); + + if (attributeNameValue) { + const errorMessage = + ngDevMode && + `Angular has detected that the \`${attributeName}\` was applied ` + + `as a binding to the <${resolvedTagName}> element${getTemplateLocationDetails(lView)}. ` + + `For security reasons, the \`${attributeName}\` can be set on the <${resolvedTagName}> element ` + + `as a static attribute only when the "attributeName" is set to \'${attributeNameValue}\'. \n` + + `To fix this, switch the \`${attributeNameValue}\` binding to a static attribute ` + + `in a template or in host bindings section.`; + + throw new RuntimeError(RuntimeErrorCode.UNSAFE_ATTRIBUTE_BINDING, errorMessage); + } + + return value; + } } - - const element = getNativeByTNode(tNode, lView) as SVGAnimateElement; - const attributeNameValue = getSecuritySensitiveSVGAnimationAttributeName( - element, - validationConfig, - ); - - if (attributeNameValue) { - const errorMessage = - ngDevMode && - `Angular has detected that the \`${attributeName}\` was applied ` + - `as a binding to the <${displayTagName}> element${getTemplateLocationDetails(lView)}. ` + - `For security reasons, the \`${attributeName}\` can be set on the <${displayTagName}> element ` + - `as a static attribute only when the "attributeName" is set to \'${attributeNameValue}\'. \n` + - `To fix this, switch the \`${attributeNameValue}\` binding to a static attribute ` + - `in a template or in host bindings section.`; - - throw new RuntimeError(RuntimeErrorCode.UNSAFE_ATTRIBUTE_BINDING, errorMessage); - } - - return value; } const errorMessage = ngDevMode && `Angular has detected that the \`${attributeName}\` was applied ` + - `as a binding to the <${displayTagName}> element${tNode ? getTemplateLocationDetails(lView) : ''}. ` + - `For security reasons, the \`${attributeName}\` can be set on the <${displayTagName}> element ` + + `as a binding to the <${resolvedTagName}> element${tNode ? getTemplateLocationDetails(lView) : ''}. ` + + `For security reasons, the \`${attributeName}\` can be set on the <${resolvedTagName}> element ` + `as a static attribute only. \n` + `To fix this, switch the \`${attributeName}\` binding to a static attribute ` + `in a template or in host bindings section.`; + throw new RuntimeError(RuntimeErrorCode.UNSAFE_ATTRIBUTE_BINDING, errorMessage); } diff --git a/packages/core/test/bundling/animations-standalone/bundle.golden_symbols.json b/packages/core/test/bundling/animations-standalone/bundle.golden_symbols.json index e676e2c62d4b..b2be4e72c09d 100644 --- a/packages/core/test/bundling/animations-standalone/bundle.golden_symbols.json +++ b/packages/core/test/bundling/animations-standalone/bundle.golden_symbols.json @@ -144,7 +144,7 @@ "MODIFIER_KEY_GETTERS", "MONKEY_PATCH_KEY_NAME", "MOVED_VIEWS", - "NAMESPACE_URIS", + "NAMESPACE_URIS2", "NATIVE", "NEXT", "NG_ANIMATING_CLASSNAME", diff --git a/packages/core/test/bundling/create_component/bundle.golden_symbols.json b/packages/core/test/bundling/create_component/bundle.golden_symbols.json index 6fa3a7cdde31..2e2d9372e6cc 100644 --- a/packages/core/test/bundling/create_component/bundle.golden_symbols.json +++ b/packages/core/test/bundling/create_component/bundle.golden_symbols.json @@ -108,7 +108,7 @@ "MODIFIER_KEY_GETTERS", "MONKEY_PATCH_KEY_NAME", "MOVED_VIEWS", - "NAMESPACE_URIS", + "NAMESPACE_URIS2", "NATIVE", "NEXT", "NG_COMP_DEF", diff --git a/packages/core/test/bundling/forms_reactive/bundle.golden_symbols.json b/packages/core/test/bundling/forms_reactive/bundle.golden_symbols.json index ad2e5ba5d806..8e9b3e23abb6 100644 --- a/packages/core/test/bundling/forms_reactive/bundle.golden_symbols.json +++ b/packages/core/test/bundling/forms_reactive/bundle.golden_symbols.json @@ -149,7 +149,7 @@ "MODIFIER_KEY_GETTERS", "MONKEY_PATCH_KEY_NAME", "MOVED_VIEWS", - "NAMESPACE_URIS", + "NAMESPACE_URIS2", "NATIVE", "NEXT", "NG_ASYNC_VALIDATORS", diff --git a/packages/core/test/bundling/forms_template_driven/bundle.golden_symbols.json b/packages/core/test/bundling/forms_template_driven/bundle.golden_symbols.json index ddffa9d94a48..58ae7bee3b4e 100644 --- a/packages/core/test/bundling/forms_template_driven/bundle.golden_symbols.json +++ b/packages/core/test/bundling/forms_template_driven/bundle.golden_symbols.json @@ -142,7 +142,7 @@ "MODIFIER_KEY_GETTERS", "MONKEY_PATCH_KEY_NAME", "MOVED_VIEWS", - "NAMESPACE_URIS", + "NAMESPACE_URIS2", "NATIVE", "NEXT", "NG_ASYNC_VALIDATORS", diff --git a/packages/core/test/bundling/hydration/bundle.golden_symbols.json b/packages/core/test/bundling/hydration/bundle.golden_symbols.json index 8f1d3e8dabe1..0a4fd5d03e36 100644 --- a/packages/core/test/bundling/hydration/bundle.golden_symbols.json +++ b/packages/core/test/bundling/hydration/bundle.golden_symbols.json @@ -168,7 +168,7 @@ "MOUSE_SPECIAL_SUPPORT", "MOVED_VIEWS", "MULTIPLIER", - "NAMESPACE_URIS", + "NAMESPACE_URIS2", "NATIVE", "NEXT", "NEXT_DEFER_BLOCK_STATE", diff --git a/packages/core/test/bundling/router/bundle.golden_symbols.json b/packages/core/test/bundling/router/bundle.golden_symbols.json index 8c5f40a12ea3..91480438f9a7 100644 --- a/packages/core/test/bundling/router/bundle.golden_symbols.json +++ b/packages/core/test/bundling/router/bundle.golden_symbols.json @@ -149,7 +149,6 @@ "MATCH_ALL_ELEMENTS", "MATH_ML_NAMESPACE", "MATH_ML_NAMESPACE2", - "MATH_ML_NAMESPACE_URI", "MATRIX_PARAM_SEGMENT_RE", "MAXIMUM_REFRESH_RERUNS", "MAXIMUM_REFRESH_RERUNS", @@ -159,6 +158,7 @@ "MONKEY_PATCH_KEY_NAME", "MOVED_VIEWS", "NAMESPACE_URIS", + "NAMESPACE_URIS2", "NATIVE", "NAVIGATION_CANCELING_ERROR", "NAVIGATION_ERROR_HANDLER", @@ -285,7 +285,6 @@ "STABILITY_WARNING_THRESHOLD", "SVG_NAMESPACE", "SVG_NAMESPACE2", - "SVG_NAMESPACE_URI", "SafeSubscriber", "SafeValueImpl", "Sanitizer", @@ -946,7 +945,6 @@ "mergeTrivialChildren", "moduleBootstrapImpl", "namedOutletsRedirect", - "namespaceUriToKey", "nativeAppendChild", "nativeAppendOrInsertBefore", "nativeInsertBefore", @@ -1129,6 +1127,7 @@ "sortActivatedRouteSnapshots", "sortByMatchingOutlets", "split", + "splitNsName", "splitQueryMultiSelectors", "squashSegmentGroup", "standardizeConfig", diff --git a/packages/core/test/bundling/standalone_bootstrap/bundle.golden_symbols.json b/packages/core/test/bundling/standalone_bootstrap/bundle.golden_symbols.json index 5455e4f8ae0b..4fb04966c36e 100644 --- a/packages/core/test/bundling/standalone_bootstrap/bundle.golden_symbols.json +++ b/packages/core/test/bundling/standalone_bootstrap/bundle.golden_symbols.json @@ -103,7 +103,7 @@ "MODIFIER_KEY_GETTERS", "MONKEY_PATCH_KEY_NAME", "MOVED_VIEWS", - "NAMESPACE_URIS", + "NAMESPACE_URIS2", "NATIVE", "NEXT", "NG_COMP_DEF", From c6112379bcf92b8df96e2aa12a3b11d4ba447402 Mon Sep 17 00:00:00 2001 From: Alan Agius <17563226+alan-agius4@users.noreply.github.com> Date: Thu, 2 Jul 2026 11:23:01 +0000 Subject: [PATCH 5/7] fixup! fix(core): account for namespaces in host binding sanitization --- .../core/test/acceptance/security_spec.ts | 467 +++++++----------- 1 file changed, 175 insertions(+), 292 deletions(-) diff --git a/packages/core/test/acceptance/security_spec.ts b/packages/core/test/acceptance/security_spec.ts index 385bc5cce671..352b51accf7c 100644 --- a/packages/core/test/acceptance/security_spec.ts +++ b/packages/core/test/acceptance/security_spec.ts @@ -885,243 +885,153 @@ describe('host binding sanitization', () => { const UNSAFE_HTML = `` + '

safe

'; const SANITIZED_HTML = '

safe

'; const resourceUrlError = /NG0904: unsafe value used in a resource URL context.*/; - let hostBindingValue = ''; - - @Component({ - selector: 'dynamic-host', - template: '', - }) - class DynamicHostComponent {} - - @Directive({ - selector: 'safe-data-carrier', - host: {'[attr.data]': 'url'}, - }) - class DataCarrierDirective { - url = hostBindingValue; - } - - @Directive({ - selector: '[href-carrier]', - host: {'[attr.href]': 'url'}, - }) - class HrefCarrierDirective { - url = hostBindingValue; - } - - @Directive({ - selector: '[xlink-href-carrier]', - host: {'[attr.xlink:href]': 'url'}, - }) - class XlinkHrefCarrierDirective { - url = hostBindingValue; - } - - @Component({ - template: ` - - - - - - `, - imports: [HrefCarrierDirective, XlinkHrefCarrierDirective], - }) - class SvgNamespaceHostBindingApp {} - - @Component({ - template: '', - imports: [HrefCarrierDirective], - }) - class MathNamespaceHostBindingApp {} - - @Component({ - selector: 'host-srcdoc-carrier', - template: '', - host: {'[attr.srcdoc]': 'srcdoc'}, - }) - class SrcdocHostComponent { - srcdoc = hostBindingValue; - } - - @Component({ - selector: 'host-action-carrier', - template: '', - host: {'[attr.action]': 'action'}, - }) - class ActionHostComponent { - action = hostBindingValue; - } - let dynamicHostElement: Element; - let dynamicHostDirective: Type; + async function expectHostBinding(options: { + tagName: string; + attrName: string; + value: string; + expected?: string; + expectedError?: RegExp; + namespace?: string; + componentSelector?: string; + }): Promise { + // Avoid duplicate selector generation. + const randomIdentifier = Math.floor(Math.random() * 100); + + const { + tagName, + attrName, + value, + expected, + expectedError, + namespace, + componentSelector = `dynamic-host-${randomIdentifier}`, + } = options; - @Component({ - template: '', - }) - class DynamicHostTestApp { - componentRef: ComponentRef; - - private appRef = inject(ApplicationRef); - private environmentInjector = inject(EnvironmentInjector); - - constructor() { - this.componentRef = createComponent(DynamicHostComponent, { - hostElement: dynamicHostElement, - environmentInjector: this.environmentInjector, - directives: [dynamicHostDirective], - }); - this.appRef.attachView(this.componentRef.hostView); + @Directive({ + selector: `[safe-data-carrier-${randomIdentifier}]`, + host: {[`[attr.${attrName}]`]: 'val'}, + }) + class CarrierDirective { + val = value; } - } - async function expectDynamicHostAttribute( - tagName: string, - attrName: string, - value: string, - expected: string, - options: {namespace?: string; directive?: Type} = {}, - ): Promise { - hostBindingValue = value; - dynamicHostElement = - options.namespace === undefined - ? document.createElement(tagName) - : document.createElementNS(options.namespace, tagName); - dynamicHostDirective = options.directive ?? DataCarrierDirective; - const fixture = TestBed.createComponent(DynamicHostTestApp); - - try { - await fixture.whenStable(); - expect(dynamicHostElement.getAttribute(attrName)).toBe(expected); - } finally { - fixture.componentInstance.componentRef.destroy(); - } - } + @Component({ + selector: componentSelector, + template: '', + }) + class DynamicComponent {} - async function expectDynamicHostResourceUrlRejection( - tagName: string, - value: string, - ): Promise { - hostBindingValue = value; - dynamicHostElement = document.createElement(tagName); - dynamicHostDirective = DataCarrierDirective; - const fixture = TestBed.createComponent(DynamicHostTestApp); + const hostElement = namespace + ? document.createElementNS(namespace, tagName) + : document.createElement(tagName); - try { - await expectAsync(fixture.whenStable()).toBeRejectedWithError(resourceUrlError); - } finally { - fixture.componentInstance.componentRef.destroy(); - } - } + let componentRef: ComponentRef | undefined; - async function expectTemplateHostAttribute( - type: Type, - selector: string, - attrName: string, - value: string, - expected: string, - ): Promise { - hostBindingValue = value; - const fixture = TestBed.createComponent(type); - await fixture.whenStable(); + @Component({ + template: '', + }) + class AppHost { + private appRef = inject(ApplicationRef); + private environmentInjector = inject(EnvironmentInjector); - const element = fixture.nativeElement.querySelector(selector) as Element; - expect(element.getAttribute(attrName)).toBe(expected); - } + constructor() { + componentRef = createComponent(DynamicComponent, { + hostElement, + environmentInjector: this.environmentInjector, + directives: [CarrierDirective], + }); + this.appRef.attachView(componentRef.hostView); + } + } - async function expectComponentHostAttribute( - type: Type, - tagName: string, - attrName: string, - value: string, - expected: string, - ): Promise { - hostBindingValue = value; - const hostElement = document.createElement(tagName); - const appRef = TestBed.inject(ApplicationRef); - const componentRef = createComponent(type, { - hostElement, - environmentInjector: TestBed.inject(EnvironmentInjector), - }); + const fixture = TestBed.createComponent(AppHost); try { - appRef.attachView(componentRef.hostView); - await appRef.whenStable(); - - expect(hostElement.getAttribute(attrName)).toBe(expected); + if (expectedError) { + await expectAsync(fixture.whenStable()).toBeRejectedWithError(expectedError); + } else { + await fixture.whenStable(); + expect(hostElement.getAttribute(attrName)).toBe(expected ?? null); + } } finally { - componentRef.destroy(); + componentRef?.destroy(); } } it('should not sanitize resource URL attribute names on non-resource concrete hosts', async () => { - await expectDynamicHostAttribute('div', 'data', HOST_BINDING_URL, HOST_BINDING_URL); - await expectDynamicHostAttribute( - 'div', - 'data', - HOST_BINDING_UNSAFE_URL, - HOST_BINDING_UNSAFE_URL, - ); + await expectHostBinding({ + tagName: 'div', + attrName: 'data', + value: HOST_BINDING_URL, + expected: HOST_BINDING_URL, + }); + await expectHostBinding({ + tagName: 'div', + attrName: 'data', + value: HOST_BINDING_UNSAFE_URL, + expected: HOST_BINDING_UNSAFE_URL, + }); }); it('should sanitize href host bindings on SVG links', async () => { - await expectTemplateHostAttribute( - SvgNamespaceHostBindingApp, - '#svg-href', - 'href', - HOST_BINDING_UNSAFE_URL, - `unsafe:${HOST_BINDING_UNSAFE_URL}`, - ); + await expectHostBinding({ + tagName: 'a', + attrName: 'href', + value: HOST_BINDING_UNSAFE_URL, + expected: `unsafe:${HOST_BINDING_UNSAFE_URL}`, + namespace: SVG_NAMESPACE_URI, + }); }); it('should not sanitize href host bindings on non-link SVG elements', async () => { - await expectTemplateHostAttribute( - SvgNamespaceHostBindingApp, - '#svg-rect', - 'href', - HOST_BINDING_UNSAFE_URL, - HOST_BINDING_UNSAFE_URL, - ); + await expectHostBinding({ + tagName: 'rect', + attrName: 'href', + value: HOST_BINDING_UNSAFE_URL, + expected: HOST_BINDING_UNSAFE_URL, + namespace: SVG_NAMESPACE_URI, + }); }); it('should sanitize xlink:href host bindings on SVG links', async () => { - await expectTemplateHostAttribute( - SvgNamespaceHostBindingApp, - '#svg-xlink-href', - 'xlink:href', - HOST_BINDING_UNSAFE_URL, - `unsafe:${HOST_BINDING_UNSAFE_URL}`, - ); + await expectHostBinding({ + tagName: 'a', + attrName: 'xlink:href', + value: HOST_BINDING_UNSAFE_URL, + expected: `unsafe:${HOST_BINDING_UNSAFE_URL}`, + namespace: SVG_NAMESPACE_URI, + }); }); it('should sanitize href host bindings on MathML elements', async () => { - await expectTemplateHostAttribute( - MathNamespaceHostBindingApp, - '#math-href', - 'href', - HOST_BINDING_UNSAFE_URL, - `unsafe:${HOST_BINDING_UNSAFE_URL}`, - ); + await expectHostBinding({ + tagName: 'mi', + attrName: 'href', + value: HOST_BINDING_UNSAFE_URL, + expected: `unsafe:${HOST_BINDING_UNSAFE_URL}`, + namespace: MATH_ML_NAMESPACE_URI, + }); }); it('should sanitize href host bindings on dynamic SVG hosts as URLs', async () => { - await expectDynamicHostAttribute( - 'a', - 'href', - HOST_BINDING_UNSAFE_URL, - `unsafe:${HOST_BINDING_UNSAFE_URL}`, - {namespace: SVG_NAMESPACE_URI, directive: HrefCarrierDirective}, - ); + await expectHostBinding({ + tagName: 'a', + attrName: 'href', + value: HOST_BINDING_UNSAFE_URL, + expected: `unsafe:${HOST_BINDING_UNSAFE_URL}`, + namespace: SVG_NAMESPACE_URI, + }); }); it('should sanitize href host bindings on dynamic MathML hosts as URLs', async () => { - await expectDynamicHostAttribute( - 'base', - 'href', - HOST_BINDING_UNSAFE_URL, - `unsafe:${HOST_BINDING_UNSAFE_URL}`, - {namespace: MATH_ML_NAMESPACE_URI, directive: HrefCarrierDirective}, - ); + await expectHostBinding({ + tagName: 'base', + attrName: 'href', + value: HOST_BINDING_UNSAFE_URL, + expected: `unsafe:${HOST_BINDING_UNSAFE_URL}`, + namespace: MATH_ML_NAMESPACE_URI, + }); }); it('should sanitize a dynamic directive host binding against the concrete host element', async () => { @@ -1172,47 +1082,42 @@ describe('host binding sanitization', () => { }); it('should not sanitize iframe-only host bindings on non-iframe concrete hosts', async () => { - await expectComponentHostAttribute( - SrcdocHostComponent, - 'div', - 'srcdoc', - UNSAFE_HTML, - UNSAFE_HTML, - ); + await expectHostBinding({ + tagName: 'div', + attrName: 'srcdoc', + value: UNSAFE_HTML, + expected: UNSAFE_HTML, + }); }); it('should not sanitize form-only URL host bindings on non-form concrete hosts', async () => { - await expectComponentHostAttribute( - ActionHostComponent, - 'div', - 'action', - HOST_BINDING_URL, - HOST_BINDING_URL, - ); - await expectComponentHostAttribute( - ActionHostComponent, - 'div', - 'action', - HOST_BINDING_UNSAFE_URL, - HOST_BINDING_UNSAFE_URL, - ); + await expectHostBinding({ + tagName: 'div', + attrName: 'action', + value: HOST_BINDING_URL, + expected: HOST_BINDING_URL, + }); + await expectHostBinding({ + tagName: 'div', + attrName: 'action', + value: HOST_BINDING_UNSAFE_URL, + expected: HOST_BINDING_UNSAFE_URL, + }); }); it('should sanitize form-only URL host bindings on form concrete hosts', async () => { - await expectComponentHostAttribute( - ActionHostComponent, - 'form', - 'action', - HOST_BINDING_URL, - HOST_BINDING_URL, - ); - await expectComponentHostAttribute( - ActionHostComponent, - 'form', - 'action', - HOST_BINDING_UNSAFE_URL, - `unsafe:${HOST_BINDING_UNSAFE_URL}`, - ); + await expectHostBinding({ + tagName: 'form', + attrName: 'action', + value: HOST_BINDING_URL, + expected: HOST_BINDING_URL, + }); + await expectHostBinding({ + tagName: 'form', + attrName: 'action', + value: HOST_BINDING_UNSAFE_URL, + expected: `unsafe:${HOST_BINDING_UNSAFE_URL}`, + }); }); it('should sanitize a host directive host binding against the concrete host element', async () => { @@ -1313,71 +1218,49 @@ describe('host binding sanitization', () => { }); it('should reject security-sensitive attribute host bindings on concrete dynamic iframe hosts', async () => { - @Directive({ - selector: 'sandbox-carrier', - host: {'[attr.sandbox]': 'sandbox'}, - }) - class SandboxCarrierDirective { - sandbox = ''; - } - - dynamicHostElement = document.createElement('iframe'); - dynamicHostDirective = SandboxCarrierDirective; - const fixture = TestBed.createComponent(DynamicHostTestApp); - - try { - await expectAsync(fixture.whenStable()).toBeRejectedWithError( + await expectHostBinding({ + tagName: 'iframe', + attrName: 'sandbox', + value: '', + expectedError: /NG0910: Angular has detected that the `sandbox` was applied as a binding to the