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 993805dc69a..9328636dca1 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 9ea142fafed..073c1eac400 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 92a17ab2e3f..54669e19872 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 f0fae600478..8ee2476afe4 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 a5ae74aaae8..d060880b015 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 30a5da12752..c031fd468c9 100644 --- a/packages/compiler/src/template/pipeline/src/ingest.ts +++ b/packages/compiler/src/template/pipeline/src/ingest.ts @@ -17,7 +17,7 @@ import * as t from '../../../render3/r3_ast'; import {DeferBlockDepsEmitMode, R3ComponentDeferMetadata} 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 { @@ -119,19 +119,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 ?? []) { @@ -140,6 +142,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 c1b9ba3c6f4..1012a3beefa 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 71c5056cc0d..50e10b53d05 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 dbd46037848..3be1c549713 100644 --- a/packages/core/src/render3/interfaces/node.ts +++ b/packages/core/src/render3/interfaces/node.ts @@ -15,6 +15,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 253b77da016..f1c969c06cd 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 a317064ac1b..0651a362d56 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', () => {