From 8ba327cb679f725d8143b5e13d37ef6d186ad587 Mon Sep 17 00:00:00 2001 From: Alan Agius <17563226+alan-agius4@users.noreply.github.com> Date: Mon, 11 May 2026 08:57:05 +0000 Subject: [PATCH 1/6] fix(core): support prefix-insensitive DOM schema lookups and compile-time i18n attribute validation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Updates `DomElementSchemaRegistry` to strip `:svg:` and `:math:` namespace prefixes from tag names before querying `SECURITY_SCHEMA` at compile-time. This allows SVG and MathML attributes to correctly match their security contexts during compilation. Also implements dynamic schema-based sanitization and validation for static i18n attributes by wrapping `` expressions directly in their core sanitizers and validators at compile-time via an optimized `switch` statement. Additionally, updates the runtime i18n parser to dynamically resolve sanitizers based on the `SECURITY_SCHEMA` registry and cleans up type assertions inside `ɵɵvalidateAttribute` to safely support evaluation when no `TNode` is selected. --- goldens/public-api/core/index.api.md | 2 + .../cli-hello-world-ivy-i18n/size.json | 2 +- packages/compiler/BUILD.bazel | 3 + packages/compiler/src/compiler.ts | 3 +- .../src/schema/dom_element_schema_registry.ts | 5 +- .../pipeline/src/phases/const_collection.ts | 5 +- .../src/phases/i18n_const_collection.ts | 32 +- .../compiler/test/schema/schema_extractor.ts | 370 ------------------ packages/core/BUILD.bazel | 1 + packages/core/src/core.ts | 2 +- packages/core/src/render3/i18n/i18n_parse.ts | 76 ++-- .../core/src/sanitization/html_sanitizer.ts | 10 - .../core/src/sanitization/sanitization.ts | 68 ++-- packages/core/src/sanitization/sanitizer.ts | 2 +- .../core/src/sanitization/schema/BUILD.bazel | 26 ++ .../schema/dom_security_schema.ts | 10 +- .../src/sanitization/{ => schema}/security.ts | 1 + packages/core/test/BUILD.bazel | 1 + .../test/linker/security_integration_spec.ts | 79 ++++ packages/core/test/render3/BUILD.bazel | 1 + .../core/test/render3/instructions_spec.ts | 2 +- .../core/test/render3/integration_spec.ts | 2 +- .../test/sanitization/sanitization_spec.ts | 2 +- 23 files changed, 243 insertions(+), 462 deletions(-) delete mode 100644 packages/compiler/test/schema/schema_extractor.ts create mode 100644 packages/core/src/sanitization/schema/BUILD.bazel rename packages/{compiler/src => core/src/sanitization}/schema/dom_security_schema.ts (95%) rename packages/core/src/sanitization/{ => schema}/security.ts (95%) diff --git a/goldens/public-api/core/index.api.md b/goldens/public-api/core/index.api.md index 1cc47bdb4250..14d77474d587 100644 --- a/goldens/public-api/core/index.api.md +++ b/goldens/public-api/core/index.api.md @@ -1769,6 +1769,8 @@ export interface SchemaMetadata { // @public export enum SecurityContext { + // (undocumented) + ATTRIBUTE_NO_BINDING = 6, // (undocumented) HTML = 1, // (undocumented) diff --git a/integration/cli-hello-world-ivy-i18n/size.json b/integration/cli-hello-world-ivy-i18n/size.json index 7edb4e7db0fc..6e17d691f055 100644 --- a/integration/cli-hello-world-ivy-i18n/size.json +++ b/integration/cli-hello-world-ivy-i18n/size.json @@ -1,4 +1,4 @@ { - "dist/main.js": 135813, + "dist/main.js": 144843, "dist/polyfills.js": 35883 } diff --git a/packages/compiler/BUILD.bazel b/packages/compiler/BUILD.bazel index c457ea2dcdfe..96788ac0d401 100644 --- a/packages/compiler/BUILD.bazel +++ b/packages/compiler/BUILD.bazel @@ -10,6 +10,9 @@ ts_project( "src/**/*.ts", ], ), + deps = [ + "//packages/core/src/sanitization/schema", + ], ) tsec_test( diff --git a/packages/compiler/src/compiler.ts b/packages/compiler/src/compiler.ts index b3a7d8c7b4b4..fb334abb7183 100644 --- a/packages/compiler/src/compiler.ts +++ b/packages/compiler/src/compiler.ts @@ -32,7 +32,8 @@ import {publishFacade} from './jit_compiler_facade'; import * as outputAst from './output/output_ast'; import {global} from './util'; -export {SECURITY_SCHEMA} from './schema/dom_security_schema'; +// @ng_package: ignore-cross-repo-import +export {SECURITY_SCHEMA} from '../../core/src/sanitization/schema/dom_security_schema'; export {CUSTOM_ELEMENTS_SCHEMA, NO_ERRORS_SCHEMA, SchemaMetadata} from './core'; export {core}; diff --git a/packages/compiler/src/schema/dom_element_schema_registry.ts b/packages/compiler/src/schema/dom_element_schema_registry.ts index 6da1bf761d32..e2abe171178b 100644 --- a/packages/compiler/src/schema/dom_element_schema_registry.ts +++ b/packages/compiler/src/schema/dom_element_schema_registry.ts @@ -9,7 +9,8 @@ import {CUSTOM_ELEMENTS_SCHEMA, NO_ERRORS_SCHEMA, SchemaMetadata, SecurityContext} from '../core'; import {isNgContainer, isNgContent} from '../ml_parser/tags'; import {dashCaseToCamelCase} from '../util'; -import {SECURITY_SCHEMA} from './dom_security_schema'; +// @ng_package: ignore-cross-repo-import +import {SECURITY_SCHEMA} from '../../../core/src/sanitization/schema/dom_security_schema'; import {ElementSchemaRegistry} from './element_schema_registry'; const BOOLEAN = 'boolean'; @@ -444,8 +445,6 @@ export class DomElementSchemaRegistry extends ElementSchemaRegistry { propName = this.getMappedPropName(propName); } - // Make sure comparisons are case insensitive, so that case differences between attribute and - // property names do not have a security impact. tagName = tagName.toLowerCase(); propName = propName.toLowerCase(); diff --git a/packages/compiler/src/template/pipeline/src/phases/const_collection.ts b/packages/compiler/src/template/pipeline/src/phases/const_collection.ts index 6451bbdfc0f1..1a5f2201a2b9 100644 --- a/packages/compiler/src/template/pipeline/src/phases/const_collection.ts +++ b/packages/compiler/src/template/pipeline/src/phases/const_collection.ts @@ -186,10 +186,7 @@ class ElementAttributes { if (value === null) { throw Error('Attribute, i18n attribute, & style element attributes must have a value'); } - if (trustedValueFn !== null) { - if (!ir.isStringLiteral(value)) { - throw Error('AssertionError: extracted attribute value should be string literal'); - } + if (trustedValueFn !== null && ir.isStringLiteral(value)) { array.push( o.taggedTemplate( trustedValueFn, diff --git a/packages/compiler/src/template/pipeline/src/phases/i18n_const_collection.ts b/packages/compiler/src/template/pipeline/src/phases/i18n_const_collection.ts index 2d481ee1ece0..b3002e6b7a52 100644 --- a/packages/compiler/src/template/pipeline/src/phases/i18n_const_collection.ts +++ b/packages/compiler/src/template/pipeline/src/phases/i18n_const_collection.ts @@ -7,6 +7,7 @@ */ import {type ConstantPool} from '../../../../constant_pool'; +import {SecurityContext} from '../../../../core'; import * as i18n from '../../../../i18n/i18n_ast'; import {mapLiteral} from '../../../../output/map_util'; import * as o from '../../../../output/output_ast'; @@ -76,6 +77,8 @@ export function collectI18nConsts(job: ComponentCompilationJob): void { // Context Xref -> Extracted Attribute Ops const extractedAttributesByI18nContext = new Map(); + // Element/ElementStart Xref -> Tag Name + const tagNamesByElement = new Map(); // Element/ElementStart Xref -> I18n Attributes config op const i18nAttributesByElement = new Map(); // Element/ElementStart Xref -> All I18n Expression ops for attrs on that target @@ -85,6 +88,9 @@ export function collectI18nConsts(job: ComponentCompilationJob): void { for (const unit of job.units) { for (const op of unit.ops()) { + if (op.kind === ir.OpKind.ElementStart || op.kind === ir.OpKind.Template) { + tagNamesByElement.set(op.xref, (op as any).tag ?? ''); + } if (op.kind === ir.OpKind.ExtractedAttribute && op.i18nContext !== null) { const attributes = extractedAttributesByI18nContext.get(op.i18nContext) ?? []; attributes.push(op); @@ -140,7 +146,31 @@ export function collectI18nConsts(job: ComponentCompilationJob): void { const attributesForMessage = extractedAttributesByI18nContext.get(op.i18nContext); if (attributesForMessage !== undefined) { for (const attr of attributesForMessage) { - attr.expression = mainVar.clone(); + let expr: o.Expression = mainVar.clone(); + const tagName = tagNamesByElement.get(attr.target) ?? ''; + switch (attr.securityContext) { + case SecurityContext.HTML: + expr = o.importExpr(Identifiers.sanitizeHtml).callFn([expr]); + break; + case SecurityContext.STYLE: + expr = o.importExpr(Identifiers.sanitizeStyle).callFn([expr]); + break; + case SecurityContext.SCRIPT: + expr = o.importExpr(Identifiers.sanitizeScript).callFn([expr]); + break; + case SecurityContext.URL: + expr = o.importExpr(Identifiers.sanitizeUrl).callFn([expr]); + break; + case SecurityContext.RESOURCE_URL: + expr = o.importExpr(Identifiers.sanitizeResourceUrl).callFn([expr]); + break; + case SecurityContext.ATTRIBUTE_NO_BINDING: + expr = o + .importExpr(Identifiers.validateAttribute) + .callFn([expr, o.literal(tagName), o.literal(attr.name)]); + break; + } + attr.expression = expr; } } } diff --git a/packages/compiler/test/schema/schema_extractor.ts b/packages/compiler/test/schema/schema_extractor.ts deleted file mode 100644 index 8ddfc00db44b..000000000000 --- a/packages/compiler/test/schema/schema_extractor.ts +++ /dev/null @@ -1,370 +0,0 @@ -/** - * @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 - */ - -const SVG_PREFIX = ':svg:'; -const MATH_PREFIX = ':math:'; - -// Element | Node interfaces -// see https://developer.mozilla.org/en-US/docs/Web/API/Element -// see https://developer.mozilla.org/en-US/docs/Web/API/Node -const ELEMENT_IF = '[Element]'; -// HTMLElement interface -// see https://developer.mozilla.org/en-US/docs/Web/API/HTMLElement -const HTMLELEMENT_IF = '[HTMLElement]'; - -const HTMLELEMENT_TAGS = - 'abbr,address,article,aside,b,bdi,bdo,cite,content,code,dd,dfn,dt,em,figcaption,figure,footer,header,hgroup,i,kbd,main,mark,nav,noscript,rb,rp,rt,rtc,ruby,s,samp,search,section,small,strong,sub,sup,u,var,wbr'; - -const ALL_HTML_TAGS = - // https://www.w3.org/TR/html5/index.html - 'a,abbr,address,area,article,aside,audio,b,base,bdi,bdo,blockquote,body,br,button,canvas,caption,cite,code,col,colgroup,content,data,datalist,dd,del,dfn,div,dl,dt,em,embed,fieldset,figcaption,figure,footer,form,h1,h2,h3,h4,h5,h6,head,header,hgroup,hr,html,i,iframe,img,input,ins,kbd,keygen,label,legend,li,link,main,map,mark,meta,meter,nav,noscript,object,ol,optgroup,option,output,p,param,pre,progress,q,rb,rp,rt,rtc,ruby,s,samp,script,search,section,select,small,source,span,strong,style,sub,sup,table,tbody,td,template,textarea,tfoot,th,thead,time,title,tr,track,u,ul,var,video,wbr,' + - // https://html.spec.whatwg.org/ - 'details,summary,menu,menuitem,geolocation'; - -// Via https://developer.mozilla.org/en-US/docs/Web/MathML -const ALL_MATH_TAGS = - 'math,maction,menclose,merror,mfenced,mfrac,mi,mmultiscripts,mn,mo,mover,mpadded,mphantom,mroot,mrow,ms,mspace,msqrt,mstyle,msub,msubsup,msup,mtable,mtd,mtext,mtr,munder,munderover,semantics'; - -// Elements missing from Chrome (HtmlUnknownElement), to be manually added -const MISSING_FROM_CHROME: {[el: string]: string[]} = { - 'data^[HTMLElement]': ['value'], - 'keygen^[HTMLElement]': ['!autofocus', 'challenge', '!disabled', 'form', 'keytype', 'name'], - // TODO(vicb): Figure out why Chrome and WhatWG do not agree on the props - // 'menu^[HTMLElement]': ['type', 'label'], - 'menuitem^[HTMLElement]': [ - 'type', - 'label', - 'icon', - '!disabled', - '!checked', - 'radiogroup', - '!default', - ], - 'summary^[HTMLElement]': [], - 'time^[HTMLElement]': ['dateTime'], - ':svg:cursor^:svg:': [], -}; - -const _G: any = - (typeof window != 'undefined' && window) || - (typeof global != 'undefined' && global) || - (typeof self != 'undefined' && self); - -const document: any = typeof _G['document'] == 'object' ? _G['document'] : null; - -export function extractSchema(): Map | null { - if (!document) return null; - const SVGGraphicsElement = _G['SVGGraphicsElement']; - if (!SVGGraphicsElement) return null; - - const element = document.createElement('video'); - const descMap: Map = new Map(); - const visited: {[name: string]: boolean} = {}; - - // HTML top level - extractProperties(Node, element, visited, descMap, ELEMENT_IF, ''); - extractProperties(Element, element, visited, descMap, ELEMENT_IF, ''); - extractProperties(HTMLElement, element, visited, descMap, HTMLELEMENT_IF, ELEMENT_IF); - extractProperties(HTMLElement, element, visited, descMap, HTMLELEMENT_TAGS, HTMLELEMENT_IF); - extractProperties(HTMLMediaElement, element, visited, descMap, 'media', HTMLELEMENT_IF); - - // SVG top level - const svgAnimation = document.createElementNS('http://www.w3.org/2000/svg', 'set'); - const svgPath = document.createElementNS('http://www.w3.org/2000/svg', 'path'); - const svgFeFuncA = document.createElementNS('http://www.w3.org/2000/svg', 'feFuncA'); - const svgGradient = document.createElementNS('http://www.w3.org/2000/svg', 'linearGradient'); - const svgText = document.createElementNS('http://www.w3.org/2000/svg', 'text'); - - const SVGAnimationElement = _G['SVGAnimationElement']; - const SVGGeometryElement = _G['SVGGeometryElement']; - const SVGComponentTransferFunctionElement = _G['SVGComponentTransferFunctionElement']; - const SVGGradientElement = _G['SVGGradientElement']; - const SVGTextContentElement = _G['SVGTextContentElement']; - const SVGTextPositioningElement = _G['SVGTextPositioningElement']; - extractProperties(SVGElement, svgText, visited, descMap, SVG_PREFIX, HTMLELEMENT_IF); - - extractProperties( - SVGGraphicsElement, - svgText, - visited, - descMap, - SVG_PREFIX + 'graphics', - SVG_PREFIX, - ); - extractProperties( - SVGAnimationElement, - svgAnimation, - visited, - descMap, - SVG_PREFIX + 'animation', - SVG_PREFIX, - ); - extractProperties( - SVGGeometryElement, - svgPath, - visited, - descMap, - SVG_PREFIX + 'geometry', - SVG_PREFIX, - ); - extractProperties( - SVGComponentTransferFunctionElement, - svgFeFuncA, - visited, - descMap, - SVG_PREFIX + 'componentTransferFunction', - SVG_PREFIX, - ); - extractProperties( - SVGGradientElement, - svgGradient, - visited, - descMap, - SVG_PREFIX + 'gradient', - SVG_PREFIX, - ); - extractProperties( - SVGTextContentElement, - svgText, - visited, - descMap, - SVG_PREFIX + 'textContent', - SVG_PREFIX + 'graphics', - ); - extractProperties( - SVGTextPositioningElement, - svgText, - visited, - descMap, - SVG_PREFIX + 'textPositioning', - SVG_PREFIX + 'textContent', - ); - - // Get all element types - const types = Object.getOwnPropertyNames(window).filter((k) => /^(HTML|SVG).*?Element$/.test(k)); - - types.sort(); - - types.forEach((type) => { - extractRecursiveProperties(visited, descMap, (window as any)[type]); - }); - - // Add elements missed by Chrome auto-detection - Object.keys(MISSING_FROM_CHROME).forEach((elHierarchy) => { - descMap.set(elHierarchy, MISSING_FROM_CHROME[elHierarchy]); - }); - - // Needed because we're running tests against some older Android versions. - if (typeof MathMLElement !== 'undefined') { - // Math top level - const math = document.createElementNS('http://www.w3.org/1998/Math/MathML', 'math'); - extractProperties(MathMLElement, math, visited, descMap, MATH_PREFIX, HTMLELEMENT_IF); - - // This script is written under the assumption that each tag has a corresponding class name, e.g. - // `` -> `SVGCircleElement` however this doesn't hold for Math elements which are all - // `MathMLElement`. Furthermore, they don't have special property names, but rather are - // configured exclusively via attributes. Register them as plain elements that inherit from - // the top-level `:math` namespace. - ALL_MATH_TAGS.split(',').forEach((tag) => - descMap.set(`${MATH_PREFIX}${tag}^${MATH_PREFIX}`, []), - ); - } - - assertNoMissingTags(descMap); - - return descMap; -} - -function assertNoMissingTags(descMap: Map): void { - const extractedTags: string[] = []; - - Array.from(descMap.keys()).forEach((key: string) => { - extractedTags.push(...key.split('|')[0].split('^')[0].split(',')); - }); - - const missingTags = [ - ...ALL_HTML_TAGS.split(','), - ...(typeof MathMLElement === 'undefined' - ? [] - : ALL_MATH_TAGS.split(',').map((tag) => MATH_PREFIX + tag)), - ].filter((tag) => !extractedTags.includes(tag)); - - if (missingTags.length) { - throw new Error(`DOM schema misses tags: ${missingTags.join(',')}`); - } -} - -function extractRecursiveProperties( - visited: {[name: string]: boolean}, - descMap: Map, - type: Function, -): string { - const name = extractName(type)!; - - if (visited[name]) { - return name; - } - - let superName: string; - switch (name) { - case ELEMENT_IF: - // ELEMENT_IF is the top most interface (Element | Node) - superName = ''; - break; - case HTMLELEMENT_IF: - superName = ELEMENT_IF; - break; - default: - superName = extractRecursiveProperties( - visited, - descMap, - type.prototype.__proto__.constructor, - ); - } - - let instance: HTMLElement | null = null; - name.split(',').forEach((tagName) => { - instance = type['name'].startsWith('SVG') - ? document.createElementNS('http://www.w3.org/2000/svg', tagName.replace(SVG_PREFIX, '')) - : document.createElement(tagName); - - let htmlType: Function; - - switch (tagName) { - case 'cite': - // interface is `HTMLQuoteElement` - htmlType = HTMLElement; - break; - default: - htmlType = type; - } - - if (!(instance instanceof htmlType)) { - throw new Error(`Tag <${tagName}> is not an instance of ${htmlType['name']}`); - } - }); - - extractProperties(type, instance, visited, descMap, name, superName); - - return name; -} - -function extractProperties( - type: Function, - instance: any, - visited: {[name: string]: boolean}, - descMap: Map, - name: string, - superName: string, -) { - if (!type) return; - - visited[name] = true; - - const fullName = name + (superName ? '^' + superName : ''); - - const props: string[] = descMap.has(fullName) ? descMap.get(fullName)! : []; - - const prototype = type.prototype; - const keys = Object.getOwnPropertyNames(prototype); - - keys.sort(); - keys.forEach((name) => { - if (name.startsWith('on')) { - props.push('*' + name.slice(2)); - } else { - const typeCh = _TYPE_MNEMONICS[typeof instance[name]]; - const descriptor = Object.getOwnPropertyDescriptor(prototype, name); - const isSetter = descriptor && descriptor.set; - if (typeCh !== void 0 && !name.startsWith('webkit') && isSetter) { - props.push(typeCh + name); - } - } - }); - - // There is no point in using `Node.nodeValue`, filter it out - descMap.set(fullName, type === Node ? props.filter((p) => p != '%nodeValue') : props); -} - -function extractName(type: Function): string | null { - let name = type['name']; - - // The polyfill @webcomponents/custom-element/src/native-shim.js overrides the - // window.HTMLElement and does not have the name property. Check if this is the - // case and if so, set the name manually. - if (name === '' && type === HTMLElement) { - name = 'HTMLElement'; - } - - switch (name) { - // see https://www.w3.org/TR/html5/index.html - // TODO(vicb): generate this map from all the element types - case 'Element': - return ELEMENT_IF; - case 'HTMLElement': - return HTMLELEMENT_IF; - case 'HTMLImageElement': - return 'img'; - case 'HTMLAnchorElement': - return 'a'; - case 'HTMLDListElement': - return 'dl'; - case 'HTMLDirectoryElement': - return 'dir'; - case 'HTMLHeadingElement': - return 'h1,h2,h3,h4,h5,h6'; - case 'HTMLModElement': - return 'ins,del'; - case 'HTMLOListElement': - return 'ol'; - case 'HTMLParagraphElement': - return 'p'; - case 'HTMLQuoteElement': - return 'q,blockquote,cite'; - case 'HTMLTableCaptionElement': - return 'caption'; - case 'HTMLTableCellElement': - return 'th,td'; - case 'HTMLTableColElement': - return 'col,colgroup'; - case 'HTMLTableRowElement': - return 'tr'; - case 'HTMLTableSectionElement': - return 'tfoot,thead,tbody'; - case 'HTMLUListElement': - return 'ul'; - case 'SVGGraphicsElement': - return SVG_PREFIX + 'graphics'; - case 'SVGMPathElement': - return SVG_PREFIX + 'mpath'; - case 'SVGSVGElement': - return SVG_PREFIX + 'svg'; - case 'SVGTSpanElement': - return SVG_PREFIX + 'tspan'; - default: - const isSVG = name.startsWith('SVG'); - if (name.startsWith('HTML') || isSVG) { - name = name.replace('HTML', '').replace('SVG', '').replace('Element', ''); - if (isSVG && name.startsWith('FE')) { - name = 'fe' + name.substring(2); - } else if (name) { - name = name.charAt(0).toLowerCase() + name.substring(1); - } - return isSVG ? SVG_PREFIX + name : name.toLowerCase(); - } - } - - return null; -} - -const _TYPE_MNEMONICS: {[type: string]: string} = { - 'string': '', - 'number': '#', - 'boolean': '!', - 'object': '%', -}; diff --git a/packages/core/BUILD.bazel b/packages/core/BUILD.bazel index c6f5576c0c43..c5a6920dd433 100644 --- a/packages/core/BUILD.bazel +++ b/packages/core/BUILD.bazel @@ -48,6 +48,7 @@ ng_project( "//packages/core/src/di/interface", "//packages/core/src/interface", "//packages/core/src/reflection", + "//packages/core/src/sanitization/schema", "//packages/core/src/util", "//packages/core/third_party/@mcp-b/webmcp-types", ], diff --git a/packages/core/src/core.ts b/packages/core/src/core.ts index d934eefdf4c4..ede7f25c3b79 100644 --- a/packages/core/src/core.ts +++ b/packages/core/src/core.ts @@ -115,7 +115,7 @@ export {createEnvironmentInjector, createNgModule} from './render3/ng_module_ref export {publishExternalGlobalUtil as ɵpublishExternalGlobalUtil} from './render3/util/global_utils'; export * from './resource'; export {Sanitizer} from './sanitization/sanitizer'; -export {SecurityContext} from './sanitization/security'; +export {SecurityContext} from './sanitization/schema/security'; export { GetTestability, setTestabilityGetter, diff --git a/packages/core/src/render3/i18n/i18n_parse.ts b/packages/core/src/render3/i18n/i18n_parse.ts index 5459e0a920d5..3ad16f372efa 100644 --- a/packages/core/src/render3/i18n/i18n_parse.ts +++ b/packages/core/src/render3/i18n/i18n_parse.ts @@ -9,18 +9,18 @@ import '../../util/ng_dev_mode'; import '../../util/ng_i18n_closure_mode'; import {XSS_SECURITY_URL} from '../../error_details_base_url'; -import { - getTemplateContent, - SENSITIVE_ATTRS, - VALID_ATTRS, - VALID_ELEMENTS, -} from '../../sanitization/html_sanitizer'; +import {getTemplateContent, VALID_ATTRS, VALID_ELEMENTS} from '../../sanitization/html_sanitizer'; import {getInertBodyHelper} from '../../sanitization/inert_body'; import {_sanitizeUrl} from '../../sanitization/url_sanitizer'; import { + ɵɵsanitizeHtml as _sanitizeHtml, + ɵɵsanitizeStyle as _sanitizeStyle, + ɵɵsanitizeScript as _sanitizeScript, + ɵɵsanitizeResourceUrl as _sanitizeResourceUrl, ɵɵvalidateAttribute as _validateAttribute, - SECURITY_SENSITIVE_ELEMENTS, } from '../../sanitization/sanitization'; +import {SECURITY_SCHEMA} from '../../sanitization/schema/dom_security_schema'; +import {SecurityContext} from '../../sanitization/schema/security'; import { assertDefined, assertEqual, @@ -392,7 +392,7 @@ export function i18nAttributesFirstPass(tView: TView, index: number, values: str previousElementIndex, attrName, countBindings(updateOpCodes), - i18nSanitizeAttribute(attrName), + i18nResolveSanitizer(attrName, previousElement.value), ); } } @@ -820,7 +820,7 @@ function walkIcuTree( newIndex, attr.name, 0, - i18nSanitizeAttribute(lowerAttrName), + i18nResolveSanitizer(lowerAttrName, tagName), ); } else { ngDevMode && @@ -831,9 +831,9 @@ function walkIcuTree( ); } } else if (VALID_ATTRS[lowerAttrName]) { - if (SENSITIVE_ATTRS[lowerAttrName]) { - // Don't sanitize, because no value is acceptable in sensitive attributes. - // Translators are not allowed to create URIs. + let val = attr.value; + const sanitizer = i18nResolveSanitizer(lowerAttrName, tagName); + if (sanitizer) { if (typeof ngDevMode !== 'undefined' && ngDevMode) { console.warn( `WARNING: ignoring unsafe attribute ` + @@ -841,9 +841,10 @@ function walkIcuTree( `(see ${XSS_SECURITY_URL})`, ); } + addCreateAttribute(create, newIndex, attr.name, 'unsafe:blocked'); } else { - addCreateAttribute(create, newIndex, attr.name, attr.value); + addCreateAttribute(create, newIndex, attr.name, val); } } else { if (typeof ngDevMode !== 'undefined' && ngDevMode) { @@ -974,32 +975,29 @@ function addCreateAttribute( create.push((newIndex << IcuCreateOpCode.SHIFT_REF) | IcuCreateOpCode.Attr, attrName, attrValue); } -/** - * Caches all keys of `SECURITY_SENSITIVE_ELEMENTS` in a Set to avoid recomputing - * or scanning them on every invocation. - */ -const SECURITY_SENSITIVE_ATTRS: ReadonlySet = /* @__PURE__ */ (() => - new Set( - Object.values(SECURITY_SENSITIVE_ELEMENTS).flatMap((attrs) => - attrs ? Object.keys(attrs) : [], - ), - ))(); - -/** - * Returns a sanitizer for the given attribute name or null if the attribute is not security sensitive. - * - * @param attrName The name of the attribute to sanitize. - * @returns The sanitizer for the given attribute name. - */ -function i18nSanitizeAttribute(attrName: string): SanitizerFn | null { +function i18nResolveSanitizer(attrName: string, tagName?: string): SanitizerFn | null { const lowerAttrName = attrName.toLowerCase(); - if (SENSITIVE_ATTRS[lowerAttrName]) { - return _sanitizeUrl; + const lowerTagName = tagName ? tagName.toLowerCase() : '*'; + const schema = SECURITY_SCHEMA(); + const schemaContext = + schema[`${lowerTagName}|${lowerAttrName}`] || + schema[`*|${lowerAttrName}`] || + SecurityContext.NONE; + + switch (schemaContext) { + case SecurityContext.HTML: + return _sanitizeHtml; + case SecurityContext.STYLE: + return _sanitizeStyle; + case SecurityContext.SCRIPT: + return _sanitizeScript; + case SecurityContext.URL: + return _sanitizeUrl; + case SecurityContext.RESOURCE_URL: + return _sanitizeResourceUrl; + case SecurityContext.ATTRIBUTE_NO_BINDING: + return _validateAttribute; + default: + return null; } - - if (SECURITY_SENSITIVE_ATTRS.has(lowerAttrName)) { - return _validateAttribute; - } - - return null; } diff --git a/packages/core/src/sanitization/html_sanitizer.ts b/packages/core/src/sanitization/html_sanitizer.ts index 1238ea8bf531..8605775e9275 100644 --- a/packages/core/src/sanitization/html_sanitizer.ts +++ b/packages/core/src/sanitization/html_sanitizer.ts @@ -116,16 +116,6 @@ export const VALID_ATTRS: BooleanRecord = merge(URI_ATTRS, HTML_ATTRS, ARIA_ATTR // don't want to preserve the content, if the elements themselves are going to be removed. const SKIP_TRAVERSING_CONTENT_IF_INVALID_ELEMENTS = tagSet('script,style,template'); -/** - * Attributes that are potential attach vectors and may need to be sanitized. - */ -export const SENSITIVE_ATTRS: BooleanRecord = merge( - URI_ATTRS, - // Note: we don't include these attributes in `URI_ATTRS`, because `URI_ATTRS` also - // determines whether an attribute should be dropped when sanitizing an HTML string. - tagSet('action,formaction,data,codebase'), -); - /** * SanitizingHtmlSerializer serializes a DOM fragment, stripping out any unsafe elements and unsafe * attributes. diff --git a/packages/core/src/sanitization/sanitization.ts b/packages/core/src/sanitization/sanitization.ts index 9b0c6b54063d..933b195eacb3 100644 --- a/packages/core/src/sanitization/sanitization.ts +++ b/packages/core/src/sanitization/sanitization.ts @@ -10,10 +10,10 @@ 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 {TNodeType} from '../render3/interfaces/node'; +import {TNode, TNodeType} from '../render3/interfaces/node'; import {RElement} from '../render3/interfaces/renderer_dom'; import {ENVIRONMENT} from '../render3/interfaces/view'; -import {getLView, getSelectedTNode} from '../render3/state'; +import {getLView, getSelectedIndex, getSelectedTNode} from '../render3/state'; import {renderStringify} from '../render3/util/stringify_utils'; import {getNativeByTNode} from '../render3/util/view_utils'; import {TrustedHTML, TrustedScript, TrustedScriptURL} from '../util/security/trusted_type_defs'; @@ -28,7 +28,7 @@ import {allowSanitizationBypassAndThrow, BypassType, unwrapSafeValue} from './by import {_sanitizeHtml as _sanitizeHtml} from './html_sanitizer'; import {enforceIframeSecurity} from './iframe_attrs_validation'; import {Sanitizer} from './sanitizer'; -import {SecurityContext} from './security'; +import {SecurityContext} from './schema/security'; import {_sanitizeUrl as _sanitizeUrl} from './url_sanitizer'; /** @@ -283,7 +283,7 @@ 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) */ -export const SECURITY_SENSITIVE_ELEMENTS: Record< +const SECURITY_SENSITIVE_ELEMENTS: Record< string, Record> | undefined > = { @@ -317,46 +317,62 @@ export function ɵɵvalidateAttribute(value: T, tagName: string, attrib 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; + } + // Leverage tNode.namespace if active, otherwise check both namespaced and base variants. - const tNode = getSelectedTNode(); const fullTagName = lowerCaseTagName[0] !== ':' && tNode?.namespace ? `:${tNode.namespace}:${lowerCaseTagName}` : lowerCaseTagName; - const validationConfig = SECURITY_SENSITIVE_ELEMENTS[fullTagName]?.[lowerCaseAttrName]; + const validationConfig = + SECURITY_SENSITIVE_ELEMENTS[fullTagName]?.[lowerCaseAttrName] ?? + (lowerCaseTagName !== '*' ? SECURITY_SENSITIVE_ELEMENTS[`:svg:${lowerCaseTagName}`]?.[lowerCaseAttrName] : undefined) ?? + (lowerCaseTagName !== '*' ? SECURITY_SENSITIVE_ELEMENTS[`:math:${lowerCaseTagName}`]?.[lowerCaseAttrName] : undefined); if (!validationConfig) { return value; } const lView = getLView(); - if (lowerCaseTagName === 'iframe') { - if (tNode?.type === TNodeType.Element) { - const element = getNativeByTNode(tNode, lView) as RElement; - enforceIframeSecurity(element as HTMLIFrameElement); - } + 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?.type === TNodeType.Element) { - const element = getNativeByTNode(tNode, lView) as SVGAnimateElement; - const attributeNameValue = element.getAttribute('attributeName'); + 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); + } + + const element = getNativeByTNode(tNode, lView) as SVGAnimateElement; + const attributeNameValue = element.getAttribute('attributeName'); - if (attributeNameValue && validationConfig.has(attributeNameValue.toLowerCase())) { - 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.`; + if (attributeNameValue && validationConfig.has(attributeNameValue.toLowerCase())) { + 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); - } + throw new RuntimeError(RuntimeErrorCode.UNSAFE_ATTRIBUTE_BINDING, errorMessage); } return value; @@ -365,7 +381,7 @@ export function ɵɵvalidateAttribute(value: T, tagName: string, attrib const errorMessage = ngDevMode && `Angular has detected that the \`${attributeName}\` was applied ` + - `as a binding to the <${displayTagName}> element${getTemplateLocationDetails(lView)}. ` + + `as a binding to the <${displayTagName}> element${tNode ? getTemplateLocationDetails(lView) : ''}. ` + `For security reasons, the \`${attributeName}\` can be set on the <${displayTagName}> element ` + `as a static attribute only. \n` + `To fix this, switch the \`${attributeName}\` binding to a static attribute ` + diff --git a/packages/core/src/sanitization/sanitizer.ts b/packages/core/src/sanitization/sanitizer.ts index af51883f85ae..f31690f42fa4 100644 --- a/packages/core/src/sanitization/sanitizer.ts +++ b/packages/core/src/sanitization/sanitizer.ts @@ -7,7 +7,7 @@ */ import {ɵɵdefineInjectable} from '../di/interface/defs'; -import {SecurityContext} from './security'; +import {SecurityContext} from './schema/security'; /** * Sanitizer is used by the views to sanitize potentially dangerous values. diff --git a/packages/core/src/sanitization/schema/BUILD.bazel b/packages/core/src/sanitization/schema/BUILD.bazel new file mode 100644 index 000000000000..62bc8e7c6a64 --- /dev/null +++ b/packages/core/src/sanitization/schema/BUILD.bazel @@ -0,0 +1,26 @@ +load("//tools:defaults.bzl", "ts_project", "tsec_test") + +package(default_visibility = ["//visibility:public"]) + +ts_project( + name = "schema", + srcs = [ + "dom_security_schema.ts", + "security.ts", + ], +) + +tsec_test( + name = "tsec_test", + target = ":schema", + tsconfig = "//packages/core:tsconfig_build", +) + +filegroup( + name = "files_for_docgen", + srcs = [ + "dom_security_schema.ts", + "security.ts", + ], + visibility = ["//visibility:public"], +) diff --git a/packages/compiler/src/schema/dom_security_schema.ts b/packages/core/src/sanitization/schema/dom_security_schema.ts similarity index 95% rename from packages/compiler/src/schema/dom_security_schema.ts rename to packages/core/src/sanitization/schema/dom_security_schema.ts index f9707eff1aee..e8c3f4c6c605 100644 --- a/packages/compiler/src/schema/dom_security_schema.ts +++ b/packages/core/src/sanitization/schema/dom_security_schema.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.dev/license */ -import {SecurityContext} from '../core'; +import {SecurityContext} from './security'; // ================================================================================================= // ================================================================================================= @@ -18,11 +18,17 @@ import {SecurityContext} from '../core'; // // ================================================================================================= -/** Map from tagName|propertyName to SecurityContext. Properties applying to all tags use '*'. */ +/** + * Map from tagName|propertyName to SecurityContext. Properties applying to all tags use '*'. + */ let _SECURITY_SCHEMA!: {[k: string]: SecurityContext}; const SVG_NAMESPACE = 'svg'; const MATH_ML_NAMESPACE = 'math'; +/** + * @remarks Keep is a copy of DOM Security Schema. + * @see [SECURITY_SCHEMA](../../../compiler/src/schema/dom_security_schema.ts) + */ export function SECURITY_SCHEMA(): {[k: string]: SecurityContext} { if (!_SECURITY_SCHEMA) { _SECURITY_SCHEMA = {}; diff --git a/packages/core/src/sanitization/security.ts b/packages/core/src/sanitization/schema/security.ts similarity index 95% rename from packages/core/src/sanitization/security.ts rename to packages/core/src/sanitization/schema/security.ts index 87e72bcd532f..75d5f85d530d 100644 --- a/packages/core/src/sanitization/security.ts +++ b/packages/core/src/sanitization/schema/security.ts @@ -22,4 +22,5 @@ export enum SecurityContext { SCRIPT = 3, URL = 4, RESOURCE_URL = 5, + ATTRIBUTE_NO_BINDING = 6, } diff --git a/packages/core/test/BUILD.bazel b/packages/core/test/BUILD.bazel index b1e52a0f0f59..cd9fd30a4936 100644 --- a/packages/core/test/BUILD.bazel +++ b/packages/core/test/BUILD.bazel @@ -41,6 +41,7 @@ ts_project( "//packages/core/src/di/interface", "//packages/core/src/interface", "//packages/core/src/reflection", + "//packages/core/src/sanitization/schema", "//packages/core/src/util", "//packages/core/testing", "//packages/localize/init", diff --git a/packages/core/test/linker/security_integration_spec.ts b/packages/core/test/linker/security_integration_spec.ts index 968501c3b384..bd01ac1a3feb 100644 --- a/packages/core/test/linker/security_integration_spec.ts +++ b/packages/core/test/linker/security_integration_spec.ts @@ -7,6 +7,8 @@ */ import {DomSanitizer} from '@angular/platform-browser'; +import {clearTranslations, loadTranslations} from '@angular/localize'; +import {computeMsgId} from '@angular/compiler'; import { ChangeDetectionStrategy, Component, @@ -282,6 +284,83 @@ describe('security integration tests', function () { }); describe('translation', () => { + afterEach(() => { + clearTranslations(); + }); + + it('should throw error on translated SVG script ResourceURL attributes', () => { + const template = ` + + + + `; + TestBed.overrideComponent(SecuredComponent, {set: {template}}); + + expect(() => TestBed.createComponent(SecuredComponent)).toThrowError( + /unsafe value used in a resource URL context/i, + ); + }); + + it('should throw error on SVG animation retargeting attributes', () => { + const template = ` + + + + + + + `; + TestBed.overrideComponent(SecuredComponent, {set: {template}}); + + expect(() => { + const fixture = TestBed.createComponent(SecuredComponent); + fixture.detectChanges(); + }).toThrowError( + /For security reasons, the `attributeName` can be set on the element as a static attribute only/i, + ); + }); + + it('should allow non-security sensitive attributes', () => { + loadTranslations({[computeMsgId('foo')]: 'bar'}); + const template = ``; + TestBed.overrideComponent(SecuredComponent, {set: {template}}); + + const fixture = TestBed.createComponent(SecuredComponent); + fixture.detectChanges(); + const element = fixture.nativeElement.querySelector('iframe'); + expect(element.getAttribute('title')).toEqual('bar'); + }); + + it('should sanitize translations of static iframe attributes', () => { + const template = ``; + TestBed.overrideComponent(SecuredComponent, {set: {template}}); + + expect(() => { + const fixture = TestBed.createComponent(SecuredComponent); + fixture.detectChanges(); + }).toThrowError( + /For security reasons, the `sandbox` can be set on the `; TestBed.overrideComponent(SecuredComponent, {set: {template}}); diff --git a/packages/core/test/render3/BUILD.bazel b/packages/core/test/render3/BUILD.bazel index 79b9e8cd284a..c20d1b248eed 100644 --- a/packages/core/test/render3/BUILD.bazel +++ b/packages/core/test/render3/BUILD.bazel @@ -30,6 +30,7 @@ ts_project( "//packages/core/rxjs-interop", "//packages/core/src/di/interface", "//packages/core/src/interface", + "//packages/core/src/sanitization/schema", "//packages/core/src/util", "//packages/core/testing", "//packages/platform-browser", diff --git a/packages/core/test/render3/instructions_spec.ts b/packages/core/test/render3/instructions_spec.ts index 3cb2ebe1bb88..74b85b1b683a 100644 --- a/packages/core/test/render3/instructions_spec.ts +++ b/packages/core/test/render3/instructions_spec.ts @@ -37,7 +37,7 @@ import { ɵɵsanitizeUrl, } from '../../src/sanitization/sanitization'; import {Sanitizer} from '../../src/sanitization/sanitizer'; -import {SecurityContext} from '../../src/sanitization/security'; +import {SecurityContext} from '../../src/sanitization/schema/security'; import {ViewFixture} from './view_fixture'; diff --git a/packages/core/test/render3/integration_spec.ts b/packages/core/test/render3/integration_spec.ts index 3af84c013f60..c1e59fff7f7f 100644 --- a/packages/core/test/render3/integration_spec.ts +++ b/packages/core/test/render3/integration_spec.ts @@ -19,7 +19,7 @@ import {TestBed} from '../../testing'; import {getLContext, readPatchedData} from '../../src/render3/context_discovery'; import {CONTEXT, HEADER_OFFSET} from '../../src/render3/interfaces/view'; import {Sanitizer} from '../../src/sanitization/sanitizer'; -import {SecurityContext} from '../../src/sanitization/security'; +import {SecurityContext} from '../../src/sanitization/schema/security'; describe('element discovery', () => { it('should only monkey-patch immediate child nodes in a component', () => { diff --git a/packages/core/test/sanitization/sanitization_spec.ts b/packages/core/test/sanitization/sanitization_spec.ts index 6264d0159206..355e20b3eae0 100644 --- a/packages/core/test/sanitization/sanitization_spec.ts +++ b/packages/core/test/sanitization/sanitization_spec.ts @@ -28,7 +28,7 @@ import { ɵɵtrustConstantHtml, ɵɵtrustConstantResourceUrl, } from '../../src/sanitization/sanitization'; -import {SecurityContext} from '../../src/sanitization/security'; +import {SecurityContext} from '../../src/sanitization/schema/security'; function fakeLView(): LView { const fake = [null, {}] as LView; From ff31495b4eca0e621c5463e3e35cc78911ccf92e Mon Sep 17 00:00:00 2001 From: Alan Agius <17563226+alan-agius4@users.noreply.github.com> Date: Tue, 12 May 2026 07:13:56 +0000 Subject: [PATCH 2/6] fixup! fix(core): support prefix-insensitive DOM schema lookups and compile-time i18n attribute validation --- .../src/template/pipeline/src/emit.ts | 2 + .../src/phases/i18n_const_collection.ts | 32 +--------- .../phases/resolve_i18n_attr_sanitizers.ts | 64 +++++++++++++++++++ 3 files changed, 67 insertions(+), 31 deletions(-) create mode 100644 packages/compiler/src/template/pipeline/src/phases/resolve_i18n_attr_sanitizers.ts diff --git a/packages/compiler/src/template/pipeline/src/emit.ts b/packages/compiler/src/template/pipeline/src/emit.ts index 00529edc9943..91fc45431029 100644 --- a/packages/compiler/src/template/pipeline/src/emit.ts +++ b/packages/compiler/src/template/pipeline/src/emit.ts @@ -75,6 +75,7 @@ import {resolveDeferDepsFns} from './phases/resolve_defer_deps_fns'; import {resolveDollarEvent} from './phases/resolve_dollar_event'; import {resolveI18nElementPlaceholders} from './phases/resolve_i18n_element_placeholders'; import {resolveI18nExpressionPlaceholders} from './phases/resolve_i18n_expression_placeholders'; +import {resolveI18nAttrSanitizers} from './phases/resolve_i18n_attr_sanitizers'; import {resolveNames} from './phases/resolve_names'; import {resolveSanitizers} from './phases/resolve_sanitizers'; import {removeSafeNavigationMigration} from './phases/safe_navigation_migration'; @@ -161,6 +162,7 @@ const phases: Phase[] = [ {kind: Kind.Tmpl, fn: resolveI18nExpressionPlaceholders}, {kind: Kind.Tmpl, fn: extractI18nMessages}, {kind: Kind.Tmpl, fn: collectI18nConsts}, + {kind: Kind.Tmpl, fn: resolveI18nAttrSanitizers}, {kind: Kind.Tmpl, fn: collectConstExpressions}, {kind: Kind.Both, fn: collectElementConsts}, {kind: Kind.Tmpl, fn: removeI18nContexts}, diff --git a/packages/compiler/src/template/pipeline/src/phases/i18n_const_collection.ts b/packages/compiler/src/template/pipeline/src/phases/i18n_const_collection.ts index b3002e6b7a52..2d481ee1ece0 100644 --- a/packages/compiler/src/template/pipeline/src/phases/i18n_const_collection.ts +++ b/packages/compiler/src/template/pipeline/src/phases/i18n_const_collection.ts @@ -7,7 +7,6 @@ */ import {type ConstantPool} from '../../../../constant_pool'; -import {SecurityContext} from '../../../../core'; import * as i18n from '../../../../i18n/i18n_ast'; import {mapLiteral} from '../../../../output/map_util'; import * as o from '../../../../output/output_ast'; @@ -77,8 +76,6 @@ export function collectI18nConsts(job: ComponentCompilationJob): void { // Context Xref -> Extracted Attribute Ops const extractedAttributesByI18nContext = new Map(); - // Element/ElementStart Xref -> Tag Name - const tagNamesByElement = new Map(); // Element/ElementStart Xref -> I18n Attributes config op const i18nAttributesByElement = new Map(); // Element/ElementStart Xref -> All I18n Expression ops for attrs on that target @@ -88,9 +85,6 @@ export function collectI18nConsts(job: ComponentCompilationJob): void { for (const unit of job.units) { for (const op of unit.ops()) { - if (op.kind === ir.OpKind.ElementStart || op.kind === ir.OpKind.Template) { - tagNamesByElement.set(op.xref, (op as any).tag ?? ''); - } if (op.kind === ir.OpKind.ExtractedAttribute && op.i18nContext !== null) { const attributes = extractedAttributesByI18nContext.get(op.i18nContext) ?? []; attributes.push(op); @@ -146,31 +140,7 @@ export function collectI18nConsts(job: ComponentCompilationJob): void { const attributesForMessage = extractedAttributesByI18nContext.get(op.i18nContext); if (attributesForMessage !== undefined) { for (const attr of attributesForMessage) { - let expr: o.Expression = mainVar.clone(); - const tagName = tagNamesByElement.get(attr.target) ?? ''; - switch (attr.securityContext) { - case SecurityContext.HTML: - expr = o.importExpr(Identifiers.sanitizeHtml).callFn([expr]); - break; - case SecurityContext.STYLE: - expr = o.importExpr(Identifiers.sanitizeStyle).callFn([expr]); - break; - case SecurityContext.SCRIPT: - expr = o.importExpr(Identifiers.sanitizeScript).callFn([expr]); - break; - case SecurityContext.URL: - expr = o.importExpr(Identifiers.sanitizeUrl).callFn([expr]); - break; - case SecurityContext.RESOURCE_URL: - expr = o.importExpr(Identifiers.sanitizeResourceUrl).callFn([expr]); - break; - case SecurityContext.ATTRIBUTE_NO_BINDING: - expr = o - .importExpr(Identifiers.validateAttribute) - .callFn([expr, o.literal(tagName), o.literal(attr.name)]); - break; - } - attr.expression = expr; + attr.expression = mainVar.clone(); } } } diff --git a/packages/compiler/src/template/pipeline/src/phases/resolve_i18n_attr_sanitizers.ts b/packages/compiler/src/template/pipeline/src/phases/resolve_i18n_attr_sanitizers.ts new file mode 100644 index 000000000000..1d6077200ef4 --- /dev/null +++ b/packages/compiler/src/template/pipeline/src/phases/resolve_i18n_attr_sanitizers.ts @@ -0,0 +1,64 @@ +/** + * @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 + */ + +import {SecurityContext} from '../../../../core'; +import * as o from '../../../../output/output_ast'; +import {Identifiers} from '../../../../render3/r3_identifiers'; +import * as ir from '../../ir'; +import {CompilationJob} from '../compilation'; + +/** + * Wraps static i18n extracted attributes in their corresponding sanitizers/validators. + */ +export function resolveI18nAttrSanitizers(job: CompilationJob): void { + const tagNamesByElement = new Map(); + + for (const unit of job.units) { + for (const op of unit.ops()) { + if (op.kind === ir.OpKind.ElementStart || op.kind === ir.OpKind.Template) { + tagNamesByElement.set(op.xref, (op as any).tag ?? ''); + } + } + } + + for (const unit of job.units) { + for (const op of unit.create) { + if ( + op.kind === ir.OpKind.ExtractedAttribute && + op.i18nContext !== null && + op.expression !== null + ) { + const tagName = tagNamesByElement.get(op.target) ?? ''; + let expr = op.expression; + switch (op.securityContext) { + case SecurityContext.HTML: + expr = o.importExpr(Identifiers.sanitizeHtml).callFn([expr]); + break; + case SecurityContext.STYLE: + expr = o.importExpr(Identifiers.sanitizeStyle).callFn([expr]); + break; + case SecurityContext.SCRIPT: + expr = o.importExpr(Identifiers.sanitizeScript).callFn([expr]); + break; + case SecurityContext.URL: + expr = o.importExpr(Identifiers.sanitizeUrl).callFn([expr]); + break; + case SecurityContext.RESOURCE_URL: + expr = o.importExpr(Identifiers.sanitizeResourceUrl).callFn([expr]); + break; + case SecurityContext.ATTRIBUTE_NO_BINDING: + expr = o + .importExpr(Identifiers.validateAttribute) + .callFn([expr, o.literal(tagName), o.literal(op.name)]); + break; + } + op.expression = expr; + } + } + } +} From 5146920c892dddf829d8064b1e930e1359a4e1fa Mon Sep 17 00:00:00 2001 From: Alan Agius <17563226+alan-agius4@users.noreply.github.com> Date: Wed, 13 May 2026 07:22:43 +0000 Subject: [PATCH 3/6] fixup! fix(core): support prefix-insensitive DOM schema lookups and compile-time i18n attribute validation --- packages/compiler/BUILD.bazel | 3 - packages/compiler/src/compiler.ts | 2 - packages/compiler/src/core.ts | 12 +- .../src/schema/dom_element_schema_registry.ts | 3 +- .../src}/schema/dom_security_schema.ts | 19 +- packages/core/BUILD.bazel | 9 +- packages/core/src/core.ts | 2 +- packages/core/src/render3/i18n/i18n_parse.ts | 3 +- .../src/sanitization/dom_security_schema.ts | 195 ++++++++++++++++++ .../core/src/sanitization/sanitization.ts | 2 +- packages/core/src/sanitization/sanitizer.ts | 2 +- .../core/src/sanitization/schema/BUILD.bazel | 26 --- .../core/src/sanitization/schema/security.ts | 26 --- packages/core/test/BUILD.bazel | 1 - packages/core/test/render3/BUILD.bazel | 1 - .../core/test/render3/instructions_spec.ts | 2 +- .../core/test/render3/integration_spec.ts | 2 +- .../test/sanitization/sanitization_spec.ts | 3 +- 18 files changed, 231 insertions(+), 82 deletions(-) rename packages/{core/src/sanitization => compiler/src}/schema/dom_security_schema.ts (93%) create mode 100644 packages/core/src/sanitization/dom_security_schema.ts delete mode 100644 packages/core/src/sanitization/schema/BUILD.bazel delete mode 100644 packages/core/src/sanitization/schema/security.ts diff --git a/packages/compiler/BUILD.bazel b/packages/compiler/BUILD.bazel index 96788ac0d401..c457ea2dcdfe 100644 --- a/packages/compiler/BUILD.bazel +++ b/packages/compiler/BUILD.bazel @@ -10,9 +10,6 @@ ts_project( "src/**/*.ts", ], ), - deps = [ - "//packages/core/src/sanitization/schema", - ], ) tsec_test( diff --git a/packages/compiler/src/compiler.ts b/packages/compiler/src/compiler.ts index fb334abb7183..60a4ebf3abee 100644 --- a/packages/compiler/src/compiler.ts +++ b/packages/compiler/src/compiler.ts @@ -32,8 +32,6 @@ import {publishFacade} from './jit_compiler_facade'; import * as outputAst from './output/output_ast'; import {global} from './util'; -// @ng_package: ignore-cross-repo-import -export {SECURITY_SCHEMA} from '../../core/src/sanitization/schema/dom_security_schema'; export {CUSTOM_ELEMENTS_SCHEMA, NO_ERRORS_SCHEMA, SchemaMetadata} from './core'; export {core}; diff --git a/packages/compiler/src/core.ts b/packages/compiler/src/core.ts index a085fa566a54..7f9f0ad0d215 100644 --- a/packages/compiler/src/core.ts +++ b/packages/compiler/src/core.ts @@ -79,16 +79,6 @@ export interface Type extends Function { } export const Type = Function; -export enum SecurityContext { - NONE = 0, - HTML = 1, - STYLE = 2, - SCRIPT = 3, - URL = 4, - RESOURCE_URL = 5, - ATTRIBUTE_NO_BINDING = 6, -} - /** * Injection flags for DI. */ @@ -331,3 +321,5 @@ export const enum AttributeMarker { */ I18n = 6, } + +export {SecurityContext} from './schema/dom_security_schema'; diff --git a/packages/compiler/src/schema/dom_element_schema_registry.ts b/packages/compiler/src/schema/dom_element_schema_registry.ts index e2abe171178b..8644877645e9 100644 --- a/packages/compiler/src/schema/dom_element_schema_registry.ts +++ b/packages/compiler/src/schema/dom_element_schema_registry.ts @@ -9,8 +9,7 @@ import {CUSTOM_ELEMENTS_SCHEMA, NO_ERRORS_SCHEMA, SchemaMetadata, SecurityContext} from '../core'; import {isNgContainer, isNgContent} from '../ml_parser/tags'; import {dashCaseToCamelCase} from '../util'; -// @ng_package: ignore-cross-repo-import -import {SECURITY_SCHEMA} from '../../../core/src/sanitization/schema/dom_security_schema'; +import {SECURITY_SCHEMA} from './dom_security_schema'; import {ElementSchemaRegistry} from './element_schema_registry'; const BOOLEAN = 'boolean'; diff --git a/packages/core/src/sanitization/schema/dom_security_schema.ts b/packages/compiler/src/schema/dom_security_schema.ts similarity index 93% rename from packages/core/src/sanitization/schema/dom_security_schema.ts rename to packages/compiler/src/schema/dom_security_schema.ts index e8c3f4c6c605..179752e79f79 100644 --- a/packages/core/src/sanitization/schema/dom_security_schema.ts +++ b/packages/compiler/src/schema/dom_security_schema.ts @@ -6,7 +6,24 @@ * found in the LICENSE file at https://angular.dev/license */ -import {SecurityContext} from './security'; +/** + * A SecurityContext marks a location that has dangerous security implications, e.g. a DOM property + * like `innerHTML` that could cause Cross Site Scripting (XSS) security bugs when improperly + * handled. + * + * See DomSanitizer for more details on security in Angular applications. + * + * @publicApi + */ +export enum SecurityContext { + NONE = 0, + HTML = 1, + STYLE = 2, + SCRIPT = 3, + URL = 4, + RESOURCE_URL = 5, + ATTRIBUTE_NO_BINDING = 6, +} // ================================================================================================= // ================================================================================================= diff --git a/packages/core/BUILD.bazel b/packages/core/BUILD.bazel index c5a6920dd433..4eeeaf056630 100644 --- a/packages/core/BUILD.bazel +++ b/packages/core/BUILD.bazel @@ -1,3 +1,4 @@ +load("@bazel_lib//lib:write_source_files.bzl", "write_source_file") load("@npm//:defs.bzl", "npm_link_all_packages") load("//adev/shared-docs/pipeline/api-gen:generate_api_docs.bzl", "generate_api_docs") load("//packages/common/locales:index.bzl", "generate_base_locale_file") @@ -48,7 +49,6 @@ ng_project( "//packages/core/src/di/interface", "//packages/core/src/interface", "//packages/core/src/reflection", - "//packages/core/src/sanitization/schema", "//packages/core/src/util", "//packages/core/third_party/@mcp-b/webmcp-types", ], @@ -166,3 +166,10 @@ genrule( outs = ["event-dispatch-contract.min.js"], cmd = "cp $< $@", ) + +write_source_file( + name = "dom_security_schema", + check_that_out_file_exists = False, + in_file = "//packages/compiler:src/schema/dom_security_schema.ts", + out_file = ":src/sanitization/dom_security_schema.ts", +) diff --git a/packages/core/src/core.ts b/packages/core/src/core.ts index ede7f25c3b79..054cac34be1d 100644 --- a/packages/core/src/core.ts +++ b/packages/core/src/core.ts @@ -115,7 +115,7 @@ export {createEnvironmentInjector, createNgModule} from './render3/ng_module_ref export {publishExternalGlobalUtil as ɵpublishExternalGlobalUtil} from './render3/util/global_utils'; export * from './resource'; export {Sanitizer} from './sanitization/sanitizer'; -export {SecurityContext} from './sanitization/schema/security'; +export {SecurityContext} from './sanitization/dom_security_schema'; export { GetTestability, setTestabilityGetter, diff --git a/packages/core/src/render3/i18n/i18n_parse.ts b/packages/core/src/render3/i18n/i18n_parse.ts index 3ad16f372efa..c622721306a7 100644 --- a/packages/core/src/render3/i18n/i18n_parse.ts +++ b/packages/core/src/render3/i18n/i18n_parse.ts @@ -19,8 +19,7 @@ import { ɵɵsanitizeResourceUrl as _sanitizeResourceUrl, ɵɵvalidateAttribute as _validateAttribute, } from '../../sanitization/sanitization'; -import {SECURITY_SCHEMA} from '../../sanitization/schema/dom_security_schema'; -import {SecurityContext} from '../../sanitization/schema/security'; +import {SECURITY_SCHEMA, SecurityContext} from '../../sanitization/dom_security_schema'; import { assertDefined, assertEqual, diff --git a/packages/core/src/sanitization/dom_security_schema.ts b/packages/core/src/sanitization/dom_security_schema.ts new file mode 100644 index 000000000000..5c6196bd2bd4 --- /dev/null +++ b/packages/core/src/sanitization/dom_security_schema.ts @@ -0,0 +1,195 @@ +/** + * @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 + */ + +/** + * A SecurityContext marks a location that has dangerous security implications, e.g. a DOM property + * like `innerHTML` that could cause Cross Site Scripting (XSS) security bugs when improperly + * handled. + * + * See DomSanitizer for more details on security in Angular applications. + * + * @publicApi + */ +export enum SecurityContext { + NONE = 0, + HTML = 1, + STYLE = 2, + SCRIPT = 3, + URL = 4, + RESOURCE_URL = 5, + ATTRIBUTE_NO_BINDING = 6, +} + +// ================================================================================================= +// ================================================================================================= +// =========== S T O P - S T O P - S T O P - S T O P - S T O P - S T O P =========== +// ================================================================================================= +// ================================================================================================= +// +// DO NOT EDIT THIS LIST OF SECURITY SENSITIVE PROPERTIES WITHOUT A SECURITY REVIEW! +// +// ================================================================================================= + +/** + * Map from tagName|propertyName to SecurityContext. Properties applying to all tags use '*'. + */ +let _SECURITY_SCHEMA!: {[k: string]: SecurityContext}; + +/** + * @remarks Keep is a copy of DOM Security Schema. + * @see [SECURITY_SCHEMA](../../../compiler/src/schema/dom_security_schema.ts) + */ +export function SECURITY_SCHEMA(): {[k: string]: SecurityContext} { + if (!_SECURITY_SCHEMA) { + _SECURITY_SCHEMA = {}; + // Case is insignificant below, all element and attribute names are lower-cased for lookup. + + registerContext(SecurityContext.HTML, ['iframe|srcdoc', '*|innerHTML', '*|outerHTML']); + registerContext(SecurityContext.STYLE, ['*|style']); + // NB: no SCRIPT contexts here, they are never allowed due to the parser stripping them. + registerContext(SecurityContext.URL, [ + '*|formAction', + 'area|href', + 'a|href', + 'a|xlink:href', + 'form|action', + + // MathML namespace + // https://crsrc.org/c/third_party/blink/renderer/core/sanitizer/sanitizer.cc;l=753-768;drc=b3eb16372dcd3317d65e9e0265015e322494edcd;bpv=1;bpt=1 + 'annotation|href', + 'annotation|xlink:href', + 'annotation-xml|href', + 'annotation-xml|xlink:href', + 'maction|href', + 'maction|xlink:href', + 'malignmark|href', + 'malignmark|xlink:href', + 'math|href', + 'math|xlink:href', + 'mroot|href', + 'mroot|xlink:href', + 'msqrt|href', + 'msqrt|xlink:href', + 'merror|href', + 'merror|xlink:href', + 'mfrac|href', + 'mfrac|xlink:href', + 'mglyph|href', + 'mglyph|xlink:href', + 'msub|href', + 'msub|xlink:href', + 'msup|href', + 'msup|xlink:href', + 'msubsup|href', + 'msubsup|xlink:href', + 'mmultiscripts|href', + 'mmultiscripts|xlink:href', + 'mprescripts|href', + 'mprescripts|xlink:href', + 'mi|href', + 'mi|xlink:href', + 'mn|href', + 'mn|xlink:href', + 'mo|href', + 'mo|xlink:href', + 'mpadded|href', + 'mpadded|xlink:href', + 'mphantom|href', + 'mphantom|xlink:href', + 'mrow|href', + 'mrow|xlink:href', + 'ms|href', + 'ms|xlink:href', + 'mspace|href', + 'mspace|xlink:href', + 'mstyle|href', + 'mstyle|xlink:href', + 'mtable|href', + 'mtable|xlink:href', + 'mtd|href', + 'mtd|xlink:href', + 'mtr|href', + 'mtr|xlink:href', + 'mtext|href', + 'mtext|xlink:href', + 'mover|href', + 'mover|xlink:href', + 'munder|href', + 'munder|xlink:href', + 'munderover|href', + 'munderover|xlink:href', + 'semantics|href', + 'semantics|xlink:href', + 'none|href', + 'none|xlink:href', + + // The below two items are safe and should be removed but they require a G3 clean-up as a small number of tests fail. + 'img|src', + 'video|src', + ]); + + registerContext(SecurityContext.RESOURCE_URL, [ + 'base|href', + 'embed|src', + 'frame|src', + 'iframe|src', + 'link|href', + 'object|codebase', + 'object|data', + 'script|src', + // The below two are for Script SVG + // See: https://developer.mozilla.org/en-US/docs/Web/API/SVGScriptElement/href + 'script|href', + 'script|xlink:href', + ]); + + // Keep this in sync with SECURITY_SENSITIVE_ELEMENTS in packages/core/src/sanitization/sanitization.ts + // The `unknown` elements refer to cases when we need to validate the input/binding in a directive (host bindings) + // and the directive can be applied to multiple different elements (with different tag names). In this case we generate + // a special instruction that an attribute might potentially be security-sensitive and defer the actual security check + // to runtime, when we apply that directive to a concrete elements, thus we can check the combination of tag+attribute + // against the set that requires sanitization. + // These are unsafe as `attributeName` can be `href` or `xlink:href` + // See: http://b/463880509#comment7 + registerContext(SecurityContext.ATTRIBUTE_NO_BINDING, [ + 'animate|attributeName', + 'animate|values', + 'animate|to', + 'animate|from', + 'set|to', + 'set|attributeName', + 'animateMotion|attributeName', + 'animateTransform|attributeName', + + 'unknown|attributeName', + 'unknown|values', + 'unknown|to', + 'unknown|from', + + 'iframe|sandbox', + 'iframe|allow', + 'iframe|allowFullscreen', + 'iframe|referrerPolicy', + 'iframe|csp', + 'iframe|fetchPriority', + + 'unknown|sandbox', + 'unknown|allow', + 'unknown|allowFullscreen', + 'unknown|referrerPolicy', + 'unknown|csp', + 'unknown|fetchPriority', + ]); + } + + return _SECURITY_SCHEMA; +} + +function registerContext(ctx: SecurityContext, specs: string[]) { + for (const spec of specs) _SECURITY_SCHEMA[spec.toLowerCase()] = ctx; +} diff --git a/packages/core/src/sanitization/sanitization.ts b/packages/core/src/sanitization/sanitization.ts index 933b195eacb3..4d96c4580511 100644 --- a/packages/core/src/sanitization/sanitization.ts +++ b/packages/core/src/sanitization/sanitization.ts @@ -28,7 +28,7 @@ import {allowSanitizationBypassAndThrow, BypassType, unwrapSafeValue} from './by import {_sanitizeHtml as _sanitizeHtml} from './html_sanitizer'; import {enforceIframeSecurity} from './iframe_attrs_validation'; import {Sanitizer} from './sanitizer'; -import {SecurityContext} from './schema/security'; +import {SecurityContext} from './dom_security_schema'; import {_sanitizeUrl as _sanitizeUrl} from './url_sanitizer'; /** diff --git a/packages/core/src/sanitization/sanitizer.ts b/packages/core/src/sanitization/sanitizer.ts index f31690f42fa4..1488c96a4160 100644 --- a/packages/core/src/sanitization/sanitizer.ts +++ b/packages/core/src/sanitization/sanitizer.ts @@ -7,7 +7,7 @@ */ import {ɵɵdefineInjectable} from '../di/interface/defs'; -import {SecurityContext} from './schema/security'; +import {SecurityContext} from './dom_security_schema'; /** * Sanitizer is used by the views to sanitize potentially dangerous values. diff --git a/packages/core/src/sanitization/schema/BUILD.bazel b/packages/core/src/sanitization/schema/BUILD.bazel deleted file mode 100644 index 62bc8e7c6a64..000000000000 --- a/packages/core/src/sanitization/schema/BUILD.bazel +++ /dev/null @@ -1,26 +0,0 @@ -load("//tools:defaults.bzl", "ts_project", "tsec_test") - -package(default_visibility = ["//visibility:public"]) - -ts_project( - name = "schema", - srcs = [ - "dom_security_schema.ts", - "security.ts", - ], -) - -tsec_test( - name = "tsec_test", - target = ":schema", - tsconfig = "//packages/core:tsconfig_build", -) - -filegroup( - name = "files_for_docgen", - srcs = [ - "dom_security_schema.ts", - "security.ts", - ], - visibility = ["//visibility:public"], -) diff --git a/packages/core/src/sanitization/schema/security.ts b/packages/core/src/sanitization/schema/security.ts deleted file mode 100644 index 75d5f85d530d..000000000000 --- a/packages/core/src/sanitization/schema/security.ts +++ /dev/null @@ -1,26 +0,0 @@ -/** - * @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 - */ - -/** - * A SecurityContext marks a location that has dangerous security implications, e.g. a DOM property - * like `innerHTML` that could cause Cross Site Scripting (XSS) security bugs when improperly - * handled. - * - * See DomSanitizer for more details on security in Angular applications. - * - * @publicApi - */ -export enum SecurityContext { - NONE = 0, - HTML = 1, - STYLE = 2, - SCRIPT = 3, - URL = 4, - RESOURCE_URL = 5, - ATTRIBUTE_NO_BINDING = 6, -} diff --git a/packages/core/test/BUILD.bazel b/packages/core/test/BUILD.bazel index cd9fd30a4936..b1e52a0f0f59 100644 --- a/packages/core/test/BUILD.bazel +++ b/packages/core/test/BUILD.bazel @@ -41,7 +41,6 @@ ts_project( "//packages/core/src/di/interface", "//packages/core/src/interface", "//packages/core/src/reflection", - "//packages/core/src/sanitization/schema", "//packages/core/src/util", "//packages/core/testing", "//packages/localize/init", diff --git a/packages/core/test/render3/BUILD.bazel b/packages/core/test/render3/BUILD.bazel index c20d1b248eed..79b9e8cd284a 100644 --- a/packages/core/test/render3/BUILD.bazel +++ b/packages/core/test/render3/BUILD.bazel @@ -30,7 +30,6 @@ ts_project( "//packages/core/rxjs-interop", "//packages/core/src/di/interface", "//packages/core/src/interface", - "//packages/core/src/sanitization/schema", "//packages/core/src/util", "//packages/core/testing", "//packages/platform-browser", diff --git a/packages/core/test/render3/instructions_spec.ts b/packages/core/test/render3/instructions_spec.ts index 74b85b1b683a..7edd0be76634 100644 --- a/packages/core/test/render3/instructions_spec.ts +++ b/packages/core/test/render3/instructions_spec.ts @@ -37,7 +37,7 @@ import { ɵɵsanitizeUrl, } from '../../src/sanitization/sanitization'; import {Sanitizer} from '../../src/sanitization/sanitizer'; -import {SecurityContext} from '../../src/sanitization/schema/security'; +import {SecurityContext} from '../../src/sanitization/dom_security_schema'; import {ViewFixture} from './view_fixture'; diff --git a/packages/core/test/render3/integration_spec.ts b/packages/core/test/render3/integration_spec.ts index c1e59fff7f7f..4ad749ab7a5f 100644 --- a/packages/core/test/render3/integration_spec.ts +++ b/packages/core/test/render3/integration_spec.ts @@ -19,7 +19,7 @@ import {TestBed} from '../../testing'; import {getLContext, readPatchedData} from '../../src/render3/context_discovery'; import {CONTEXT, HEADER_OFFSET} from '../../src/render3/interfaces/view'; import {Sanitizer} from '../../src/sanitization/sanitizer'; -import {SecurityContext} from '../../src/sanitization/schema/security'; +import {SecurityContext} from '../../src/sanitization/dom_security_schema'; describe('element discovery', () => { it('should only monkey-patch immediate child nodes in a component', () => { diff --git a/packages/core/test/sanitization/sanitization_spec.ts b/packages/core/test/sanitization/sanitization_spec.ts index 355e20b3eae0..6558cfce647a 100644 --- a/packages/core/test/sanitization/sanitization_spec.ts +++ b/packages/core/test/sanitization/sanitization_spec.ts @@ -6,7 +6,6 @@ * found in the LICENSE file at https://angular.dev/license */ -import {SECURITY_SCHEMA} from '@angular/compiler'; import {ENVIRONMENT, LView} from '../../src/render3/interfaces/view'; import {enterView, leaveView} from '../../src/render3/state'; @@ -28,7 +27,7 @@ import { ɵɵtrustConstantHtml, ɵɵtrustConstantResourceUrl, } from '../../src/sanitization/sanitization'; -import {SecurityContext} from '../../src/sanitization/schema/security'; +import {SECURITY_SCHEMA, SecurityContext} from '../../src/sanitization/dom_security_schema'; function fakeLView(): LView { const fake = [null, {}] as LView; From 2e98e52130d6e25ee326c223d0e9c99bdcbc95b4 Mon Sep 17 00:00:00 2001 From: Alan Agius <17563226+alan-agius4@users.noreply.github.com> Date: Tue, 19 May 2026 07:46:09 +0000 Subject: [PATCH 4/6] fixup! fix(core): support prefix-insensitive DOM schema lookups and compile-time i18n attribute validation --- .../src/sanitization/dom_security_schema.ts | 219 ++++++++---------- 1 file changed, 100 insertions(+), 119 deletions(-) diff --git a/packages/core/src/sanitization/dom_security_schema.ts b/packages/core/src/sanitization/dom_security_schema.ts index 5c6196bd2bd4..179752e79f79 100644 --- a/packages/core/src/sanitization/dom_security_schema.ts +++ b/packages/core/src/sanitization/dom_security_schema.ts @@ -39,6 +39,8 @@ export enum SecurityContext { * Map from tagName|propertyName to SecurityContext. Properties applying to all tags use '*'. */ let _SECURITY_SCHEMA!: {[k: string]: SecurityContext}; +const SVG_NAMESPACE = 'svg'; +const MATH_ML_NAMESPACE = 'math'; /** * @remarks Keep is a copy of DOM Security Schema. @@ -49,103 +51,74 @@ export function SECURITY_SCHEMA(): {[k: string]: SecurityContext} { _SECURITY_SCHEMA = {}; // Case is insignificant below, all element and attribute names are lower-cased for lookup. - registerContext(SecurityContext.HTML, ['iframe|srcdoc', '*|innerHTML', '*|outerHTML']); - registerContext(SecurityContext.STYLE, ['*|style']); + registerContext(SecurityContext.HTML, /** Namespace */ undefined, [ + ['iframe', ['srcdoc']], + ['*', ['innerHTML', 'outerHTML']], + ]); + registerContext(SecurityContext.STYLE, /** Namespace */ undefined, [['*', ['style']]]); // NB: no SCRIPT contexts here, they are never allowed due to the parser stripping them. - registerContext(SecurityContext.URL, [ - '*|formAction', - 'area|href', - 'a|href', - 'a|xlink:href', - 'form|action', + registerContext(SecurityContext.URL, /** Namespace */ undefined, [ + ['*', ['formAction']], + ['area', ['href']], + ['a', ['href', 'xlink:href']], + ['form', ['action']], + + // The below two items are safe and should be removed but they require a G3 clean-up as a small number of tests fail. + ['img', ['src']], + ['video', ['src']], + ]); + registerContext(SecurityContext.URL, MATH_ML_NAMESPACE, [ // MathML namespace // https://crsrc.org/c/third_party/blink/renderer/core/sanitizer/sanitizer.cc;l=753-768;drc=b3eb16372dcd3317d65e9e0265015e322494edcd;bpv=1;bpt=1 - 'annotation|href', - 'annotation|xlink:href', - 'annotation-xml|href', - 'annotation-xml|xlink:href', - 'maction|href', - 'maction|xlink:href', - 'malignmark|href', - 'malignmark|xlink:href', - 'math|href', - 'math|xlink:href', - 'mroot|href', - 'mroot|xlink:href', - 'msqrt|href', - 'msqrt|xlink:href', - 'merror|href', - 'merror|xlink:href', - 'mfrac|href', - 'mfrac|xlink:href', - 'mglyph|href', - 'mglyph|xlink:href', - 'msub|href', - 'msub|xlink:href', - 'msup|href', - 'msup|xlink:href', - 'msubsup|href', - 'msubsup|xlink:href', - 'mmultiscripts|href', - 'mmultiscripts|xlink:href', - 'mprescripts|href', - 'mprescripts|xlink:href', - 'mi|href', - 'mi|xlink:href', - 'mn|href', - 'mn|xlink:href', - 'mo|href', - 'mo|xlink:href', - 'mpadded|href', - 'mpadded|xlink:href', - 'mphantom|href', - 'mphantom|xlink:href', - 'mrow|href', - 'mrow|xlink:href', - 'ms|href', - 'ms|xlink:href', - 'mspace|href', - 'mspace|xlink:href', - 'mstyle|href', - 'mstyle|xlink:href', - 'mtable|href', - 'mtable|xlink:href', - 'mtd|href', - 'mtd|xlink:href', - 'mtr|href', - 'mtr|xlink:href', - 'mtext|href', - 'mtext|xlink:href', - 'mover|href', - 'mover|xlink:href', - 'munder|href', - 'munder|xlink:href', - 'munderover|href', - 'munderover|xlink:href', - 'semantics|href', - 'semantics|xlink:href', - 'none|href', - 'none|xlink:href', + ['annotation', ['href', 'xlink:href']], + ['annotation-xml', ['href', 'xlink:href']], + ['maction', ['href', 'xlink:href']], + ['malignmark', ['href', 'xlink:href']], + ['math', ['href', 'xlink:href']], + ['mroot', ['href', 'xlink:href']], + ['msqrt', ['href', 'xlink:href']], + ['merror', ['href', 'xlink:href']], + ['mfrac', ['href', 'xlink:href']], + ['mglyph', ['href', 'xlink:href']], + ['msub', ['href', 'xlink:href']], + ['msup', ['href', 'xlink:href']], + ['msubsup', ['href', 'xlink:href']], + ['mmultiscripts', ['href', 'xlink:href']], + ['mprescripts', ['href', 'xlink:href']], + ['mi', ['href', 'xlink:href']], + ['mn', ['href', 'xlink:href']], + ['mo', ['href', 'xlink:href']], + ['mpadded', ['href', 'xlink:href']], + ['mphantom', ['href', 'xlink:href']], + ['mrow', ['href', 'xlink:href']], + ['ms', ['href', 'xlink:href']], + ['mspace', ['href', 'xlink:href']], + ['mstyle', ['href', 'xlink:href']], + ['mtable', ['href', 'xlink:href']], + ['mtd', ['href', 'xlink:href']], + ['mtr', ['href', 'xlink:href']], + ['mtext', ['href', 'xlink:href']], + ['mover', ['href', 'xlink:href']], + ['munder', ['href', 'xlink:href']], + ['munderover', ['href', 'xlink:href']], + ['semantics', ['href', 'xlink:href']], + ['none', ['href', 'xlink:href']], + ]); - // The below two items are safe and should be removed but they require a G3 clean-up as a small number of tests fail. - 'img|src', - 'video|src', + registerContext(SecurityContext.RESOURCE_URL, /** Namespace */ undefined, [ + ['base', ['href']], + ['embed', ['src']], + ['frame', ['src']], + ['iframe', ['src']], + ['link', ['href']], + ['object', ['codebase', 'data']], ]); - registerContext(SecurityContext.RESOURCE_URL, [ - 'base|href', - 'embed|src', - 'frame|src', - 'iframe|src', - 'link|href', - 'object|codebase', - 'object|data', - 'script|src', - // The below two are for Script SVG - // See: https://developer.mozilla.org/en-US/docs/Web/API/SVGScriptElement/href - 'script|href', - 'script|xlink:href', + // The below are for Script SVG + // See: https://developer.mozilla.org/en-US/docs/Web/API/SVGScriptElement/href + registerContext(SecurityContext.RESOURCE_URL, SVG_NAMESPACE, [ + ['script', ['src', 'href', 'xlink:href']], ]); // Keep this in sync with SECURITY_SENSITIVE_ELEMENTS in packages/core/src/sanitization/sanitization.ts @@ -156,40 +129,48 @@ export function SECURITY_SCHEMA(): {[k: string]: SecurityContext} { // against the set that requires sanitization. // These are unsafe as `attributeName` can be `href` or `xlink:href` // See: http://b/463880509#comment7 - registerContext(SecurityContext.ATTRIBUTE_NO_BINDING, [ - 'animate|attributeName', - 'animate|values', - 'animate|to', - 'animate|from', - 'set|to', - 'set|attributeName', - 'animateMotion|attributeName', - 'animateTransform|attributeName', - - 'unknown|attributeName', - 'unknown|values', - 'unknown|to', - 'unknown|from', - - 'iframe|sandbox', - 'iframe|allow', - 'iframe|allowFullscreen', - 'iframe|referrerPolicy', - 'iframe|csp', - 'iframe|fetchPriority', + registerContext(SecurityContext.ATTRIBUTE_NO_BINDING, SVG_NAMESPACE, [ + ['animate', ['attributeName', 'values', 'to', 'from']], + ['set', ['to', 'attributeName']], + ['animateMotion', ['attributeName']], + ['animateTransform', ['attributeName']], + ]); - 'unknown|sandbox', - 'unknown|allow', - 'unknown|allowFullscreen', - 'unknown|referrerPolicy', - 'unknown|csp', - 'unknown|fetchPriority', + registerContext(SecurityContext.ATTRIBUTE_NO_BINDING, /** Namespace */ undefined, [ + [ + 'unknown', + [ + 'attributeName', + 'values', + 'to', + 'from', + 'sandbox', + 'allow', + 'allowFullscreen', + 'referrerPolicy', + 'csp', + 'fetchPriority', + ], + ], + ['iframe', ['sandbox', 'allow', 'allowFullscreen', 'referrerPolicy', 'csp', 'fetchPriority']], ]); } return _SECURITY_SCHEMA; } -function registerContext(ctx: SecurityContext, specs: string[]) { - for (const spec of specs) _SECURITY_SCHEMA[spec.toLowerCase()] = ctx; +function registerContext( + ctx: SecurityContext, + namespace: string | undefined, + specs: readonly [tagName: string, attributeNames: readonly string[]][], +): void { + for (const [element, attributeNames] of specs) { + let tagName = + namespace && element !== '*' && element !== 'unknown' ? `:${namespace}:${element}` : element; + tagName = tagName.toLowerCase(); + + for (const attr of attributeNames) { + _SECURITY_SCHEMA[`${tagName}|${attr.toLowerCase()}`] = ctx; + } + } } From 0b90bcb71ef9246a468868e0cc176509cea4826b Mon Sep 17 00:00:00 2001 From: Alan Agius <17563226+alan-agius4@users.noreply.github.com> Date: Tue, 19 May 2026 11:27:35 +0000 Subject: [PATCH 5/6] fixup! fix(core): support prefix-insensitive DOM schema lookups and compile-time i18n attribute validation --- .../phases/resolve_i18n_attr_sanitizers.ts | 13 +++++++++- packages/core/src/render3/i18n/i18n_apply.ts | 24 ++++++++++++------- packages/core/src/render3/i18n/i18n_parse.ts | 16 ++++++++++--- .../core/src/sanitization/sanitization.ts | 9 +++---- 4 files changed, 44 insertions(+), 18 deletions(-) diff --git a/packages/compiler/src/template/pipeline/src/phases/resolve_i18n_attr_sanitizers.ts b/packages/compiler/src/template/pipeline/src/phases/resolve_i18n_attr_sanitizers.ts index 1d6077200ef4..6b4252be9d60 100644 --- a/packages/compiler/src/template/pipeline/src/phases/resolve_i18n_attr_sanitizers.ts +++ b/packages/compiler/src/template/pipeline/src/phases/resolve_i18n_attr_sanitizers.ts @@ -11,6 +11,7 @@ import * as o from '../../../../output/output_ast'; import {Identifiers} from '../../../../render3/r3_identifiers'; import * as ir from '../../ir'; import {CompilationJob} from '../compilation'; +import {MATH_ML_NAMESPACE, SVG_NAMESPACE} from '../namespaces'; /** * Wraps static i18n extracted attributes in their corresponding sanitizers/validators. @@ -21,7 +22,17 @@ export function resolveI18nAttrSanitizers(job: CompilationJob): void { for (const unit of job.units) { for (const op of unit.ops()) { if (op.kind === ir.OpKind.ElementStart || op.kind === ir.OpKind.Template) { - tagNamesByElement.set(op.xref, (op as any).tag ?? ''); + let tag = op.tag ?? ''; + switch (op.namespace) { + case ir.Namespace.SVG: + tag = `:${SVG_NAMESPACE}:${tag}`; + break; + case ir.Namespace.Math: + tag = `:${MATH_ML_NAMESPACE}:${tag}`; + break; + } + + tagNamesByElement.set(op.xref, tag); } } } diff --git a/packages/core/src/render3/i18n/i18n_apply.ts b/packages/core/src/render3/i18n/i18n_apply.ts index b2e160541972..96745d19fbb6 100644 --- a/packages/core/src/render3/i18n/i18n_apply.ts +++ b/packages/core/src/render3/i18n/i18n_apply.ts @@ -49,8 +49,10 @@ import { } from '../dom_node_manipulation'; import { getBindingIndex, + getSelectedIndex, isInSkipHydrationBlock, lastNodeWasCreated, + setSelectedIndex, wasLastNodeCreated, } from '../state'; import {renderStringify} from '../util/stringify_utils'; @@ -442,14 +444,20 @@ function applyUpdateOpCodes( sanitizeFn, ); } else { - setPropertyAndInputs( - tNodeOrTagName, - lView, - propName, - value, - lView[RENDERER], - sanitizeFn, - ); + const prevSelectedIndex = getSelectedIndex(); + setSelectedIndex(nodeIndex); + try { + setPropertyAndInputs( + tNodeOrTagName, + lView, + propName, + value, + lView[RENDERER], + sanitizeFn, + ); + } finally { + setSelectedIndex(prevSelectedIndex); + } } break; case I18nUpdateOpCode.Text: diff --git a/packages/core/src/render3/i18n/i18n_parse.ts b/packages/core/src/render3/i18n/i18n_parse.ts index c622721306a7..28a8441ea60b 100644 --- a/packages/core/src/render3/i18n/i18n_parse.ts +++ b/packages/core/src/render3/i18n/i18n_parse.ts @@ -385,13 +385,16 @@ export function i18nAttributesFirstPass(tView: TView, index: number, values: str // the compiler treats static i18n attributes as regular attribute bindings. // Since this may not be the first i18n attribute on this element we need to pass in how // many previous bindings there have already been. + const tagName = previousElement.namespace + ? `:${previousElement.namespace}:${previousElement.value}` + : previousElement.value; generateBindingUpdateOpCodes( updateOpCodes, message, previousElementIndex, attrName, countBindings(updateOpCodes), - i18nResolveSanitizer(attrName, previousElement.value), + i18nResolveSanitizer(attrName, tagName), ); } } @@ -811,6 +814,13 @@ 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; if (hasBinding) { if (VALID_ATTRS.hasOwnProperty(lowerAttrName)) { generateBindingUpdateOpCodes( @@ -819,7 +829,7 @@ function walkIcuTree( newIndex, attr.name, 0, - i18nResolveSanitizer(lowerAttrName, tagName), + i18nResolveSanitizer(lowerAttrName, tagNameWithNamespace), ); } else { ngDevMode && @@ -831,7 +841,7 @@ function walkIcuTree( } } else if (VALID_ATTRS[lowerAttrName]) { let val = attr.value; - const sanitizer = i18nResolveSanitizer(lowerAttrName, tagName); + const sanitizer = i18nResolveSanitizer(lowerAttrName, tagNameWithNamespace); if (sanitizer) { if (typeof ngDevMode !== 'undefined' && ngDevMode) { console.warn( diff --git a/packages/core/src/sanitization/sanitization.ts b/packages/core/src/sanitization/sanitization.ts index 4d96c4580511..6976e46ee3e4 100644 --- a/packages/core/src/sanitization/sanitization.ts +++ b/packages/core/src/sanitization/sanitization.ts @@ -25,11 +25,11 @@ import { } from '../util/security/trusted_types_bypass'; import {allowSanitizationBypassAndThrow, BypassType, unwrapSafeValue} from './bypass'; -import {_sanitizeHtml as _sanitizeHtml} from './html_sanitizer'; +import {_sanitizeHtml} from './html_sanitizer'; import {enforceIframeSecurity} from './iframe_attrs_validation'; import {Sanitizer} from './sanitizer'; import {SecurityContext} from './dom_security_schema'; -import {_sanitizeUrl as _sanitizeUrl} from './url_sanitizer'; +import {_sanitizeUrl} from './url_sanitizer'; /** * An `html` sanitizer which converts untrusted `html` **string** into trusted string by removing @@ -329,10 +329,7 @@ export function ɵɵvalidateAttribute(value: T, tagName: string, attrib ? `:${tNode.namespace}:${lowerCaseTagName}` : lowerCaseTagName; - const validationConfig = - SECURITY_SENSITIVE_ELEMENTS[fullTagName]?.[lowerCaseAttrName] ?? - (lowerCaseTagName !== '*' ? SECURITY_SENSITIVE_ELEMENTS[`:svg:${lowerCaseTagName}`]?.[lowerCaseAttrName] : undefined) ?? - (lowerCaseTagName !== '*' ? SECURITY_SENSITIVE_ELEMENTS[`:math:${lowerCaseTagName}`]?.[lowerCaseAttrName] : undefined); + let validationConfig = SECURITY_SENSITIVE_ELEMENTS[fullTagName]?.[lowerCaseAttrName]; if (!validationConfig) { return value; From a58c1cd785b96c6f7c7bfcac9dc77266e22e1786 Mon Sep 17 00:00:00 2001 From: Alan Agius <17563226+alan-agius4@users.noreply.github.com> Date: Tue, 19 May 2026 11:41:00 +0000 Subject: [PATCH 6/6] fixup! fix(core): support prefix-insensitive DOM schema lookups and compile-time i18n attribute validation --- packages/core/src/sanitization/sanitization.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/core/src/sanitization/sanitization.ts b/packages/core/src/sanitization/sanitization.ts index 6976e46ee3e4..58f3ad3d42a8 100644 --- a/packages/core/src/sanitization/sanitization.ts +++ b/packages/core/src/sanitization/sanitization.ts @@ -329,7 +329,7 @@ export function ɵɵvalidateAttribute(value: T, tagName: string, attrib ? `:${tNode.namespace}:${lowerCaseTagName}` : lowerCaseTagName; - let validationConfig = SECURITY_SENSITIVE_ELEMENTS[fullTagName]?.[lowerCaseAttrName]; + const validationConfig = SECURITY_SENSITIVE_ELEMENTS[fullTagName]?.[lowerCaseAttrName]; if (!validationConfig) { return value;