fix(core): sanitize host bindings on concrete hosts by SkyZeroZx · Pull Request #69558 · angular/angular · GitHub

Host binding sanitization previously used the declaring directive or component selector to choose a compile-time security context. The same host binding can execute on a different concrete element through hostDirectives, inherited host bindings, dynamic directives, or createComponent hostElement usage.

Compute host binding security contexts against possible concrete hosts and defer URL versus ResourceURL selection to runtime when necessary. Resolve dynamic root host TNodes to their native tag before sanitizer and security-sensitive attribute checks.

Fixes #69550

More context https://issuetracker.google.com/u/1/issues/513926480

I took a deeper look at the implementation and identified two edge-case issues related to how security contexts are merged and resolved for dynamic/generic hosts:

1. Over-sanitization of benign custom properties on generic hosts

Because calcHostBindingSecurityContexts merges contexts across all possible DOM elements when a directive's selector doesn't specify a concrete tag, a property like [attr.data] (which is a RESOURCE_URL on <object> but just NONE on a generic <div>) results in a context array of [SecurityContext.RESOURCE_URL, SecurityContext.NONE].

The isUrlOrResourceUrlSecurityContext function correctly evaluates this to true and instructs the compiler to emit the ɵɵsanitizeUrlOrResourceUrl runtime sanitizer.
However, at runtime, if the concrete element happens to be a <div>, getUrlSanitizer in sanitization.ts falls back to ɵɵsanitizeUrl (because div is not in the RESOURCE_MAP for data). As a result, a completely safe string bound to a custom [attr.data] on a <div> (e.g., 'javascript:something') will be unexpectedly over-sanitized and prefixed with unsafe:, mutating benign data attributes.

2. Inconsistent fallback handling for mixed non-URL contexts

In calcHostBindingSecurityContexts, the fallback logic drops the SecurityContext.NONE context if other contexts are present:

  if (
    (hasConcreteHostNoneContext && concreteHostNonNoneCount > 0) ||
    // ...
  ) {
    return concreteHostNonNoneContexts;
  }

If a property requires a completely different sanitizer (like [attr.srcdoc] which maps to SecurityContext.HTML on an iframe and NONE elsewhere), the compiler drops NONE and correctly hardcodes ɵɵsanitizeHtml for the binding. While this works safely for srcdoc, it silently ignores the fact that the property is supposed to be unsanitized (NONE) on other tags, leading to unconditional HTML sanitization across all tags.

Additionally, if a custom schema or future DOM property mapped an attribute to [SecurityContext.RESOURCE_URL, SecurityContext.SCRIPT], isUrlOrResourceUrlSecurityContext would evaluate to false and cause getOnlySecurityContext() to throw a compilation error, as it doesn't gracefully handle mixed contexts outside of the hardcoded URL/RESOURCE_URL scenario.

1. Over-sanitization of benign custom properties on generic hosts

Because calcHostBindingSecurityContexts merges contexts across all possible DOM elements when a directive's selector doesn't specify a concrete tag, a property like [attr.data] (which is a RESOURCE_URL on <object> but just NONE on a generic <div>) results in a context array of [SecurityContext.RESOURCE_URL, SecurityContext.NONE].

Good catch, in that case I think we possibly need to generate a new instruction to sanitize host bindings, since I tried to make the fewest possible changes, which is the solution I proposed. But I agree this is an edge case that we have to consider.

Although I also think it would increase the bundle size. It seems we could use dom_security_schema.ts and avoid generating a new instruction, even if that means pulling in some additional pieces in general. Since this is a security issue, we could always revisit it later.

If we agree, we could review it, although in my experimentation I found that this increases the bundle generated specifically in the router when bringing in specific elements from the sanitization schema through the use of a[href] in RouterLink

I'm removing the script case since it's not possible to write a script in any way with the latest fixes.
See
GHSA-692r-grfm-v8x7
#69551

I updated

The concrete cases are now handled like this:

[attr.data]
- <div data> keeps both http://server/asset and javascript:custom-data unchanged.
- <object data> still rejects as ResourceURL.
[attr.action]
- <div action> keeps both values unchanged.
- <form action> keeps the safe URL and sanitizes javascript:....
[attr.srcdoc]
- <div srcdoc> keeps the value unchanged.
- <iframe srcdoc> sanitizes HTML.

I also changed the URL runtime helper to return null when the concrete tag/property pair has no URL or ResourceURL context, instead of falling back to ɵɵsanitizeUrl.

For truly ambiguous mixed non-URL contexts, like a hypothetical [RESOURCE_URL, SCRIPT], I left the compiler throwing through getOnlySecurityContext(). Since, as I understand it, this is a legacy behavior that currently works this way, I believe it would fall outside the scope of this PR.

Nice work here! The logic looks good, but I’d like to spend a bit more time reviewing the unit tests to see if we can make them easier to follow, I can be just me, but I personally am having quite a hard time to follow them.

I also noticed quite a bit of redundant code in sanitization.ts. To save some time from going back and forth, I'll push a quick commit to your branch to clean that up.

I fixed this in my commit, but for visibility, the toLowerCase here is incorrect. Namespaces URIs are case sensitive.

https://www.w3.org/TR/xml-names/#:~:text=The%20URI%20references%20below%20are%20also%20all,names%20is%20strongly%20discouraged.%203%20Declaring%20Namespaces.

LGTM

Reviewed-for: fw-security

@JeanMeche & @alan-agius4 I think it's ready this PR, or would we need something else before adding it to the merge queue?

Host binding sanitization previously used the declaring directive or component selector to choose a compile-time security context. The same host binding can execute on a different concrete element through hostDirectives, inherited host bindings, dynamic directives, or createComponent hostElement usage. Compute host binding security contexts against possible concrete hosts and defer URL versus ResourceURL selection to runtime when necessary. Resolve dynamic root host TNodes to their native tag before sanitizer and security-sensitive attribute checks. Fixes angular#69550

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.

This PR was merged into the repository. The changes were merged into the following branches:

main: f57d5d5
22.0.x: b4b4a64
22.1.x: 091456a

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

Host binding sanitization previously used the declaring directive or component selector to choose a compile-time security context. The same host binding can execute on a different concrete element through hostDirectives, inherited host bindings, dynamic directives, or createComponent hostElement usage. Compute host binding security contexts against possible concrete hosts and defer URL versus ResourceURL selection to runtime when necessary. Resolve dynamic root host TNodes to their native tag before sanitizer and security-sensitive attribute checks. Fixes #69550 PR Close #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

Host binding sanitization previously used the declaring directive or component selector to choose a compile-time security context. The same host binding can execute on a different concrete element through hostDirectives, inherited host bindings, dynamic directives, or createComponent hostElement usage. Compute host binding security contexts against possible concrete hosts and defer URL versus ResourceURL selection to runtime when necessary. Resolve dynamic root host TNodes to their native tag before sanitizer and security-sensitive attribute checks. Fixes #69550 PR Close #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

@alan-agius4
Q: Given that this could be treated as an unexpected bypass of Angular's sanitizer, should a security advisor possibly be generated?

@SkyZeroZx, yes.

GHSA-hh8m-fm6v-7cvg

@alan-agius4 It appears the GHSA-hh8m-fm6v-7cvg omitted the version that includes the patch in v20.3.28 It could be updated.

A tangential question, It might also deserve its own advisor for the case of fix(http): run root interceptors in the terminal request chain according to what was mentioned in #69777

On the other hand;

For some particular reason, did they not have an advisor? (Personally, I would think the impact was very similar or identical to other HttpTransferCache advisors)

@alan-agius4 I'm sorry to bother you again, but I think the advisor should be updated with the patch version.

@alan-agius4 It appears the GHSA-hh8m-fm6v-7cvg omitted the version that includes the patch in v20.3.28 It could be updated.

A tangential question, It might also deserve its own advisor for the case of fix(http): run root interceptors in the terminal request chain according to what was mentioned in #69777

On the other hand;

skip transfer cache for credentialed requests and uncacheable requests/responses

fix(http): prevent caching of responses with Set-Cookie headers

For some particular reason, did they not have an advisor? (Personally, I would think the impact was very similar or identical to other HttpTransferCache advisors)

Thanks for checking. Here is how we're classifying these:

fix(http): run root interceptors in the terminal request chain #69778 (withRequestsMadeViaParent): Yes, this warrants a security advisory which I will be working on.
skip transfer cache for credentialed requests and uncacheable request/responses #69017 (Fetch credentials & Cache-Control): The fetch credentials fix is an extension of GHSA-q6f4-qqrg-jv6x. The Cache-Control support is considered defense-in-depth hardening.
fix(http): prevent caching of responses with Set-Cookie headers #69385 (Set-Cookie): We consider this defense-in-depth hardening rather than a CVE. TransferState only serializes response bodies (it never replays Set-Cookie headers), so checking Set-Cookie is a heuristic to avoid caching potentially session-specific payloads.

angular-robot Bot added the area: core Issues related to the framework runtime label Jun 29, 2026

ngbot Bot added this to the Backlog milestone Jun 29, 2026

SkyZeroZx marked this pull request as ready for review June 29, 2026 14:31

pullapprove Bot requested review from alan-agius4, josephperrott and kirjs June 29, 2026 14:31

alan-agius4 reviewed Jun 30, 2026

View reviewed changes

Comment thread packages/compiler/src/template/pipeline/src/phases/resolve_sanitizers.ts

Comment thread packages/compiler/src/template/pipeline/src/ingest.ts

pullapprove Bot requested a review from alan-agius4 June 30, 2026 07:14

alan-agius4 requested review from AndrewKushnir and removed request for kirjs June 30, 2026 07:16

alan-agius4 added action: review The PR is still awaiting reviews from at least one requested reviewer target: patch This PR is targeted for the next patch release labels Jun 30, 2026

SkyZeroZx force-pushed the fix-host-binding-sanitization branch from 027cd32 to b527d55 Compare June 30, 2026 14:36

JeanMeche reviewed Jun 30, 2026

View reviewed changes

SkyZeroZx force-pushed the fix-host-binding-sanitization branch from b527d55 to 3936609 Compare June 30, 2026 21:26

SkyZeroZx commented Jun 30, 2026

View reviewed changes

Comment thread packages/core/test/sanitization/sanitization_spec.ts

AndrewKushnir removed their request for review July 1, 2026 01:25

alan-agius4 reviewed Jul 1, 2026

View reviewed changes

pullapprove Bot requested a review from alan-agius4 July 1, 2026 08:17

SkyZeroZx commented Jul 1, 2026

View reviewed changes

Comment thread packages/core/src/sanitization/sanitization.ts Outdated

alan-agius4 reviewed Jul 2, 2026

View reviewed changes

pullapprove Bot requested a review from alan-agius4 July 2, 2026 08:28

alan-agius4 reviewed Jul 2, 2026

View reviewed changes

pullapprove Bot requested a review from alan-agius4 July 2, 2026 08:40

alan-agius4 force-pushed the fix-host-binding-sanitization branch from c4895b9 to d119c78 Compare July 2, 2026 08:50

alan-agius4 added the action: global presubmit The PR is in need of a google3 global presubmit label Jul 2, 2026

alan-agius4 approved these changes Jul 2, 2026

View reviewed changes

JeanMeche added action: merge The PR is ready for merge by the caretaker and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Jul 28, 2026

SkyZeroZx force-pushed the fix-host-binding-sanitization branch from 805b371 to cddd521 Compare July 28, 2026 18:44

angular-robot Bot added the area: forms label Jul 28, 2026

SkyZeroZx and others added 7 commits July 28, 2026 13:45

fix(core): account for namespaces in host binding sanitization

fef7173

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.

fixup! fix(core): account for namespaces in host binding sanitization

35f895f

fixup! fix(core): account for namespaces in host binding sanitization

3c721ba

fixup! fix(core): account for namespaces in host binding sanitization

c611237

fixup! fix(core): account for namespaces in host binding sanitization

c1b7f72

fixup! fix(core): account for namespaces in host binding sanitization

777c258

SkyZeroZx force-pushed the fix-host-binding-sanitization branch from cddd521 to 777c258 Compare July 28, 2026 18:46

This was referenced Jul 28, 2026

[Backport 21.2.X] fix(core): sanitize host bindings on concrete hosts #69985

Merged

[Backport 20.3.X] fix(core): sanitize host bindings on concrete hosts #69986

Merged

alxhub closed this in d06e374 Jul 29, 2026

SkyZeroZx deleted the fix-host-binding-sanitization branch August 20, 2026 03:16


		expect(() => ɵɵsanitizeUrlOrResourceUrl('http://server', 'iframe', 'SRC')).toThrowError(ERROR);

		expect(ɵɵsanitizeUrlOrResourceUrl('javascript:true', 'ScRiPt', 'xLiNk:HrEf')).toEqual(

+              }
+              function namespaceUriToKey(namespaceUri: string | null | undefined): string | null {
+                switch (namespaceUri?.toLowerCase()) {

fix(core): sanitize host bindings on concrete hosts by SkyZeroZx · Pull Request #69558 · angular/angular · GitHub

Uh oh!

fix(core): sanitize host bindings on concrete hosts - #69558

fix(core): sanitize host bindings on concrete hosts#69558SkyZeroZx wants to merge 7 commits intoangular:mainangular/angular:mainfrom SkyZeroZx:fix-host-binding-sanitizationSkyZeroZx/angular:fix-host-binding-sanitizationCopy head branch name to clipboard

Conversation

SkyZeroZx commented Jun 29, 2026

Uh oh!

Uh oh!

Uh oh!

JeanMeche left a comment

Choose a reason for hiding this comment

1. Over-sanitization of benign custom properties on generic hosts

2. Inconsistent fallback handling for mixed non-URL contexts

Uh oh!

SkyZeroZx commented Jun 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

1. Over-sanitization of benign custom properties on generic hosts

Uh oh!

SkyZeroZx Jun 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SkyZeroZx commented Jun 30, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

alan-agius4 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alan-agius4 Jul 2, 2026

Choose a reason for hiding this comment

Uh oh!

alan-agius4 left a comment

Choose a reason for hiding this comment

Uh oh!

SkyZeroZx commented Jul 28, 2026

Uh oh!

alxhub commented Jul 29, 2026

Uh oh!

SkyZeroZx commented Aug 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alan-agius4 commented Aug 18, 2026

Uh oh!

alan-agius4 commented Aug 18, 2026

Uh oh!

SkyZeroZx commented Aug 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

SkyZeroZx commented Aug 20, 2026

Uh oh!

alan-agius4 commented Aug 20, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

fix(core): sanitize host bindings on concrete hosts#69558
SkyZeroZx wants to merge 7 commits into
angular:mainfrom
SkyZeroZx:fix-host-binding-sanitization

SkyZeroZx commented Jun 30, 2026 •
edited
Loading

SkyZeroZx Jun 30, 2026 •
edited
Loading

alan-agius4 left a comment •
edited
Loading

SkyZeroZx commented Aug 17, 2026 •
edited
Loading

SkyZeroZx commented Aug 19, 2026 •
edited
Loading