Skip to content

fix(compiler): sanitize href/xlink:href attributes of any element… #69191

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
JeanMeche wants to merge 1 commit into
base: main
Choose a base branch
from
+28 −40
Draft

fix(compiler): sanitize href/xlink:href attributes of any element… #69191

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions packages/compiler/src/schema/dom_element_schema_registry.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -455,10 +455,16 @@ export class DomElementSchemaRegistry extends ElementSchemaRegistry {

const normalizedTag = normalizeTagName(tagName);
propName = propName.toLowerCase();
let nsWildcardTag: string | undefined;
Copy link
Copy Markdown
Contributor

@alan-agius4 alan-agius4 Jun 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We already have a function to get the namespace normalizeTagName. Also in this case maybe do not use normalizeTagName to avoid combining the namespace and splitting it multiple times.

if (normalizedTag.startsWith(':')) {
const lastColonIndex = normalizedTag.lastIndexOf(':');
nsWildcardTag = `${normalizedTag.slice(0, lastColonIndex + 1)}*`;
}

const securitySchema = SECURITY_SCHEMA();
Copy link
Copy Markdown
Contributor

@alan-agius4 alan-agius4 Jun 5, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This logic should probably be moved into a function in dom_security_schema.ts.

This is because the same logic in also present in i18n_parser and they should be kept in sync with a shared function it's easier to avoid updating one place and not the other.

Something like

getSecurityContextFromSchema(tagName, attributeName, namespace): SecurityContext

const ctx =
securitySchema[normalizedTag + '|' + propName] ??
(nsWildcardTag !== undefined ? securitySchema[nsWildcardTag + '|' + propName] : undefined) ??
securitySchema['*|' + propName] ??
SecurityContext.NONE;

Expand Down
44 changes: 5 additions & 39 deletions packages/compiler/src/schema/dom_security_schema.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,43 +68,7 @@ export function SECURITY_SCHEMA(): {[k: string]: SecurityContext} {
['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', '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']],
]);
registerContext(SecurityContext.URL, MATH_ML_NAMESPACE, [['*', ['href', 'xlink:href']]]);
Copy link
Copy Markdown
Contributor

@alan-agius4 alan-agius4 Jun 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NIT: can you leave the previous comment (Personally it's quite useful to check every now and than)

// https://crsrc.org/c/third_party/blink/renderer/core/sanitizer/sanitizer.cc;l=753-768;drc=b3eb16372dcd3317d65e9e0265015e322494edcd;bpv=1;bpt=1


registerContext(SecurityContext.RESOURCE_URL, /** Namespace */ undefined, [
['base', ['href']],
Expand Down Expand Up @@ -161,8 +125,10 @@ function registerContext(
specs: readonly [tagName: string, attributeNames: readonly string[]][],
): void {
for (const [element, attributeNames] of specs) {
let tagName =
namespace && element !== '*' && element !== 'unknown' ? `:${namespace}:${element}` : element;
let tagName = element;
if (namespace && element !== 'unknown') {
tagName = `:${namespace}:${element}`;
}
tagName = tagName.toLowerCase();

for (const attr of attributeNames) {
Expand Down
18 changes: 17 additions & 1 deletion packages/compiler/test/schema/dom_element_schema_registry_spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,12 @@
* found in the LICENSE file at https://angular.dev/license
*/

import {CUSTOM_ELEMENTS_SCHEMA, NO_ERRORS_SCHEMA, SecurityContext} from '../../src/core';
import {
_ATTR_TO_PROP,
DomElementSchemaRegistry,
SCHEMA,
} from '../../src/schema/dom_element_schema_registry';
import {CUSTOM_ELEMENTS_SCHEMA, NO_ERRORS_SCHEMA, SecurityContext} from '../../src/core';

import {Element} from '../../src/ml_parser/ast';
import {HtmlParser} from '../../src/ml_parser/html_parser';
Expand Down Expand Up @@ -174,6 +174,22 @@ If 'onAnything' is a directive input, make sure the directive is imported by the
expect(registry.securityContext(':svg:a', 'xlink:href', false)).toBe(SecurityContext.URL);
expect(registry.securityContext(':svg:a', 'href', true)).toBe(SecurityContext.URL);
expect(registry.securityContext(':svg:a', 'xlink:href', true)).toBe(SecurityContext.URL);

// MathML link attributes
expect(registry.securityContext(':math:math', 'href', false)).toBe(SecurityContext.URL);
expect(registry.securityContext(':math:math', 'xlink:href', false)).toBe(SecurityContext.URL);
expect(registry.securityContext(':math:math', 'href', true)).toBe(SecurityContext.URL);
expect(registry.securityContext(':math:math', 'xlink:href', true)).toBe(SecurityContext.URL);
// Making sure we're have a wild card match for MathML elements with the correct security context
expect(registry.securityContext(':math:foobar', 'href', true)).toBe(SecurityContext.URL);
expect(registry.securityContext(':math:foobar', 'xlink:href', true)).toBe(SecurityContext.URL);
expect(registry.securityContext(':math:foobar', 'href', false)).toBe(SecurityContext.URL);
expect(registry.securityContext(':math:foobar', 'xlink:href', false)).toBe(SecurityContext.URL);
// Ensure MathML wildcard does not apply outside of the MathML namespace.
expect(registry.securityContext(':svg:foobar', 'href', false)).toBe(SecurityContext.NONE);
expect(registry.securityContext(':svg:foobar', 'xlink:href', false)).toBe(SecurityContext.NONE);

expect(registry.securityContext('p', 'href', false)).toBe(SecurityContext.NONE);
});

it('should detect properties on namespaced elements', () => {
Expand Down
Loading