Visitar URL original
fix(core): account for namespaces in host binding sanitization (#69558) · angular/angular@091456a · GitHub
Skip to content

Commit 091456a

Browse files
SkyZeroZxalxhub
authored andcommitted
fix(core): account for namespaces in host binding sanitization (#69558)
Make runtime URL sanitizer selection namespace-aware so SVG and MathML host bindings match the security schema. Cover SVG href/xlink:href and MathML href host binding cases, including dynamic hostElement resolution. PR Close #69558
1 parent 23cf1a8 commit 091456a

12 files changed

Lines changed: 358 additions & 391 deletions

File tree

‎packages/compiler/src/schema/dom_security_schema.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,8 +56,8 @@ type SecuritySchema = Record<
5656
>;
5757

5858
let _SECURITY_SCHEMA!: SecuritySchema;
59-
const SVG_NAMESPACE = 'svg';
60-
const MATH_ML_NAMESPACE = 'math';
59+
export const SVG_NAMESPACE = 'svg';
60+
export const MATH_ML_NAMESPACE = 'math';
6161
const NO_NAMESPACE = '';
6262
const MATCH_ALL_ELEMENTS = '*';
6363
const createNullObj = () => Object.create(null);

‎packages/compiler/src/template/pipeline/src/phases/resolve_sanitizers.ts‎

Lines changed: 23 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,10 @@ export function resolveSanitizers(job: CompilationJob): void {
5858
case ir.OpKind.DomProperty:
5959
case ir.OpKind.TwoWayProperty:
6060
let sanitizerFn: o.ExternalReference | null = null;
61-
if (isUrlOrResourceUrlSecurityContext(op.securityContext)) {
61+
if (
62+
Array.isArray(op.securityContext) &&
63+
hasCompositeUrlSecurityContext(op.securityContext)
64+
) {
6265
// When the host element isn't known, attributes such as `href`, `src`, `data`,
6366
// `action`, and `codebase` may be part of multiple security contexts. In this case we
6467
// use a special sanitization function and select the actual behavior at runtime based
@@ -76,13 +79,7 @@ export function resolveSanitizers(job: CompilationJob): void {
7679
}
7780
}
7881

79-
function isUrlOrResourceUrlSecurityContext(
80-
securityContext: SecurityContext | SecurityContext[],
81-
): boolean {
82-
if (!Array.isArray(securityContext)) {
83-
return false;
84-
}
85-
82+
function hasCompositeUrlSecurityContext(securityContext: SecurityContext[]): boolean {
8683
let hasUrlContext = false;
8784
let hasResourceUrlContext = false;
8885
let hasNoneContext = false;
@@ -115,18 +112,23 @@ function isUrlOrResourceUrlSecurityContext(
115112
function getOnlySecurityContext(
116113
securityContext: SecurityContext | SecurityContext[],
117114
): SecurityContext {
118-
if (Array.isArray(securityContext)) {
119-
const nonNoneSecurityContexts = securityContext.filter(
120-
(context) => context !== SecurityContext.NONE,
121-
);
122-
if (nonNoneSecurityContexts.length > 1) {
123-
// TODO: What should we do here? TDB just took the first one, but this feels like something we
124-
// would want to know about and create a special case for like we did for Url/ResourceUrl. My
125-
// guess is that, outside of the Url/ResourceUrl case, this never actually happens. If there
126-
// do turn out to be other cases, throwing an error until we can address it feels safer.
127-
throw Error(`AssertionError: Ambiguous security context`);
128-
}
129-
return nonNoneSecurityContexts[0] || SecurityContext.NONE;
115+
if (!Array.isArray(securityContext)) {
116+
return securityContext;
117+
}
118+
119+
if (securityContext.length < 2) {
120+
return securityContext[0] ?? SecurityContext.NONE;
121+
}
122+
123+
const nonNoneSecurityContexts = securityContext.filter(
124+
(context) => context !== SecurityContext.NONE,
125+
);
126+
if (nonNoneSecurityContexts.length > 1) {
127+
// TODO: What should we do here? TDB just took the first one, but this feels like something we
128+
// would want to know about and create a special case for like we did for Url/ResourceUrl. My
129+
// guess is that, outside of the Url/ResourceUrl case, this never actually happens. If there
130+
// do turn out to be other cases, throwing an error until we can address it feels safer.
131+
throw Error(`AssertionError: Ambiguous security context`);
130132
}
131-
return securityContext;
133+
return nonNoneSecurityContexts[0] ?? SecurityContext.NONE;
132134
}

‎packages/core/src/render3/component_ref.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import {ComponentRef as AbstractComponentRef} from '../linker/component_factory'
2121
import {createElementRef, ElementRef} from '../linker/element_ref';
2222
import {NgModuleRef} from '../linker/ng_module_factory';
2323
import {RendererFactory2} from '../render/api';
24+
import {MATH_ML_NAMESPACE, SVG_NAMESPACE} from '../sanitization/dom_security_schema';
2425
import {Sanitizer} from '../sanitization/sanitizer';
2526

2627
import {attachPatchData} from './context_discovery';
@@ -56,7 +57,6 @@ import {
5657
TVIEW,
5758
TViewType,
5859
} from './interfaces/view';
59-
import {MATH_ML_NAMESPACE, SVG_NAMESPACE} from './namespaces';
6060

6161
import {ProfilerEvent} from '../../primitives/devtools';
6262
import {TracingService} from '../application/tracing';

‎packages/core/src/render3/i18n/i18n_parse.ts‎

Lines changed: 7 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,8 @@ import {
7373
setTIcu,
7474
setTNodeInsertBeforeIndex,
7575
} from './i18n_util';
76+
import {splitNsName} from '../util/tags';
77+
import {NAMESPACE_URIS} from '../namespaces';
7678

7779
const BINDING_REGEXP = /�(\d+):?\d*�/gi;
7880
const ICU_REGEXP = /({\s*�\d+:?\d*�\s*,\s*\S{6}\s*,[\s\S]*})/gi;
@@ -658,7 +660,7 @@ function parseICUBlock(pattern: string): IcuExpression {
658660

659661
const parts = i18nParseTextIntoPartsAndICU(pattern) as string[];
660662
// Looking for (key block)+ sequence. One of the keys has to be "other".
661-
for (let pos = 0; pos < parts.length; ) {
663+
for (let pos = 0; pos < parts.length;) {
662664
let key = parts[pos++].trim();
663665
if (icuType === IcuType.plural) {
664666
// Key can be "=x", we just want "x"
@@ -814,13 +816,10 @@ function walkIcuTree(
814816
const attr = elAttrs.item(i)!;
815817
const lowerAttrName = attr.name.toLowerCase();
816818
const hasBinding = !!attr.value.match(BINDING_REGEXP);
817-
const elementNS = element.namespaceURI;
818-
const tagNameWithNamespace =
819-
elementNS === 'http://www.w3.org/2000/svg'
820-
? `:svg:${tagName}`
821-
: elementNS === 'http://www.w3.org/1998/Math/MathML'
822-
? `:math:${tagName}`
823-
: tagName;
819+
const namespaceUri = element.namespaceURI;
820+
const namespace = namespaceUri && NAMESPACE_URIS[namespaceUri];
821+
const tagNameWithNamespace = namespace ? `:${namespace}:${tagName}` : tagName;
822+
824823
if (hasBinding) {
825824
if (VALID_ATTRS.hasOwnProperty(lowerAttrName)) {
826825
generateBindingUpdateOpCodes(
@@ -984,24 +983,6 @@ function addCreateAttribute(
984983
create.push((newIndex << IcuCreateOpCode.SHIFT_REF) | IcuCreateOpCode.Attr, attrName, attrValue);
985984
}
986985

987-
function splitNsName(elementName: string, fatal: boolean = true): [string | null, string] {
988-
if (elementName[0] != ':') {
989-
return [null, elementName];
990-
}
991-
992-
const colonIndex = elementName.indexOf(':', 1);
993-
994-
if (colonIndex === -1) {
995-
if (fatal) {
996-
throw new Error(`Unsupported format "${elementName}" expecting ":namespace:name"`);
997-
} else {
998-
return [null, elementName];
999-
}
1000-
}
1001-
1002-
return [elementName.slice(1, colonIndex), elementName.slice(colonIndex + 1)];
1003-
}
1004-
1005986
function i18nResolveSanitizer(attrName: string, tagName?: string): SanitizerFn | null {
1006987
let schemaContext: SecurityContext;
1007988

‎packages/core/src/render3/namespaces.ts‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,5 +6,9 @@
66
* found in the LICENSE file at https://angular.dev/license
77
*/
88

9-
export const SVG_NAMESPACE = 'svg';
10-
export const MATH_ML_NAMESPACE = 'math';
9+
import {MATH_ML_NAMESPACE, SVG_NAMESPACE} from '../sanitization/dom_security_schema';
10+
11+
export const NAMESPACE_URIS: Record<string, string> = {
12+
'http://www.w3.org/2000/svg': SVG_NAMESPACE,
13+
'http://www.w3.org/1998/Math/MathML': MATH_ML_NAMESPACE,
14+
};

‎packages/core/src/render3/state.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
*/
88

99
import {InternalInjectFlags} from '../di/interface/injector';
10+
import {MATH_ML_NAMESPACE, SVG_NAMESPACE} from '../sanitization/dom_security_schema';
1011
import {
1112
assertDefined,
1213
assertEqual,
@@ -31,7 +32,6 @@ import {
3132
TView,
3233
TViewType,
3334
} from './interfaces/view';
34-
import {MATH_ML_NAMESPACE, SVG_NAMESPACE} from './namespaces';
3535
import {getTNode, walkUpViews} from './util/view_utils';
3636

3737
/**
Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
/**
2+
* @license
3+
* Copyright Google LLC All Rights Reserved.
4+
*
5+
* Use of this source code is governed by an MIT-style license that can be
6+
* found in the LICENSE file at https://angular.dev/license
7+
*/
8+
9+
/**
10+
* Splits an element name into its namespace and local name.
11+
*
12+
* @param elementName The element name to split, in the format ":namespace:name".
13+
* @param fatal If true, throws an error if the element name is not in the correct format.
14+
* @returns A tuple containing the namespace and local name.
15+
*/
16+
export function splitNsName(elementName: string, fatal: boolean = true): [string | null, string] {
17+
if (elementName[0] != ':') {
18+
return [null, elementName];
19+
}
20+
21+
const colonIndex = elementName.indexOf(':', 1);
22+
23+
if (colonIndex === -1) {
24+
if (fatal) {
25+
throw new Error(`Unsupported format "${elementName}" expecting ":namespace:name"`);
26+
} else {
27+
return [null, elementName];
28+
}
29+
}
30+
31+
return [elementName.slice(1, colonIndex), elementName.slice(colonIndex + 1)];
32+
}

‎packages/core/src/sanitization/dom_security_schema.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,8 +56,8 @@ type SecuritySchema = Record<
5656
>;
5757

5858
let _SECURITY_SCHEMA!: SecuritySchema;
59-
const SVG_NAMESPACE = 'svg';
60-
const MATH_ML_NAMESPACE = 'math';
59+
export const SVG_NAMESPACE = 'svg';
60+
export const MATH_ML_NAMESPACE = 'math';
6161
const NO_NAMESPACE = '';
6262
const MATCH_ALL_ELEMENTS = '*';
6363
const createNullObj = () => Object.create(null);

0 commit comments

Comments
 (0)