Skip to content

fix(compiler): disallow i18n on svg animation attributes (XSS)#68560

Closed
Hexix23 wants to merge 1 commit into
angular:mainfrom
Hexix23:fix-i18n-svg-animation-attrs-v3
Closed

fix(compiler): disallow i18n on svg animation attributes (XSS)#68560
Hexix23 wants to merge 1 commit into
angular:mainfrom
Hexix23:fix-i18n-svg-animation-attrs-v3

Conversation

@Hexix23
Copy link
Copy Markdown

@Hexix23 Hexix23 commented May 4, 2026

This fix addresses a security issue in the Angular compiler i18n metadata pass.

Summary: Angular i18n SVG retargets safe animation into executable href.

The change follows a Google OSS VRP review that asked for the issue to be
handled through the Angular GitHub workflow for disclosure and remediation.

i18n-* attributes were rejected when they targeted Trusted Types sinks, but
they were not rejected when they targeted SVG animation attributes that Angular's
own DOM security schema marks as SecurityContext.ATTRIBUTE_NO_BINDING.

This allowed localized static SVG animation attributes to bypass the same
security-sensitive attribute restrictions that Angular already enforces for
dynamic bindings. SVG animation attributes such as attributeName and to can
retarget an otherwise safe animation into a security-sensitive link target at
runtime.

Changes:

  • Check translated SVG animation attributes against Angular's DOM security
    schema in addition to the existing Trusted Types sink check.
  • Reject i18n-* metadata for SVG animation attributes whose security context
    is SecurityContext.ATTRIBUTE_NO_BINDING.
  • Normalize namespaced element names before security-context lookup, so SVG
    elements such as :svg:set are checked against the same schema entries as
    dynamic bindings.
  • Add regression coverage for translated SVG animation attributes.

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • angular.dev application / infrastructure changes
  • Other... Please describe:

What is the current behavior?

Issue Number: N/A

Angular's i18n metadata visitor only rejects translated attributes when
isTrustedTypesSink(tagName, attrName) returns true.

SVG animation attributes that are marked in dom_security_schema.ts as
SecurityContext.ATTRIBUTE_NO_BINDING are not rejected by the i18n metadata
pass. As a result, localized static SVG animation attributes can be emitted even
though Angular blocks equivalent dynamic bindings.

What is the new behavior?

Translated SVG animation attributes are rejected when Angular's DOM security
schema marks them as SecurityContext.ATTRIBUTE_NO_BINDING.

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

Validated locally with:

  • bazelisk test //packages/core/test:test --test_filter='security integration tests translation'
  • bazelisk test //packages/compiler/test:test --test_filter='i18n'
  • npx --yes prettier@3.8.0 --check packages/compiler/src/render3/view/i18n/meta.ts packages/core/test/linker/security_integration_spec.ts
  • git diff --check -- packages/compiler/src/render3/view/i18n/meta.ts packages/core/test/linker/security_integration_spec.ts

@angular-robot angular-robot Bot added the area: compiler Issues related to `ngc`, Angular's template compiler label May 4, 2026
@ngbot ngbot Bot added this to the Backlog milestone May 4, 2026
josephperrott
josephperrott approved these changes May 4, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@josephperrott josephperrott left a comment

Choose a reason for hiding this comment

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

LGTM

Reviewed-for: fw-security

@Hexix23
fix(compiler): disallow i18n on svg animation attributes
2b5d59e 
Reject translated SVG animation attributes so localization cannot retarget safe animations to href values.
@Hexix23 Hexix23 force-pushed the fix-i18n-svg-animation-attrs-v3 branch from 73e30b5 to 2b5d59e Compare May 4, 2026 23:31
@Hexix23 Hexix23 changed the title fix(compiler): disallow i18n on svg animation attributes fix(compiler): disallow i18n on svg animation attributes (XSS) May 5, 2026
@alan-agius4
Copy link
Copy Markdown
Contributor

alan-agius4 commented May 5, 2026

Woah, looks like you've opened a lot of issues/PRs recently. While we appreciate contributions from the community, triaging and reviewing a large influx of content in a short time period takes time away from other ongoing projects. As a result, we're closing these issues/PRs to maintain the team's focus.

Note that this is not necessarily a rejection of the goals or direction of any of these contributions in particular, so much as a reflection of the team's current capacity and priorities.

You are welcome to open a smaller subset of issues/PRs in accordance with our policy focused on the most important and impactful contributions and we will do our best to prioritize a response as soon as possible.

@alan-agius4 alan-agius4 closed this May 5, 2026
@Hexix23
Copy link
Copy Markdown
Author

Hexix23 commented May 5, 2026

Woah, looks like you've opened a lot of issues/PRs recently. While we appreciate contributions from the community, triaging and reviewing a large influx of content in a short time period takes time away from other ongoing projects. As a result, we're closing these issues/PRs to maintain the team's focus.

Note that this is not necessarily a rejection of the goals or direction of any of these contributions in particular, so much as a reflection of the team's current capacity and priorities.

You are welcome to open a smaller subset of issues/PRs in accordance with our policy focused on the most important and impactful contributions and we will do our best to prioritize a response as soon as possible.

Hi Alan,

All the reports I have opened were originally submitted through your VRP platform. They have been sitting there for a while without a response, or some were closed instructing me to report them this way (although, according to the documentation, I understood the standard procedure was the other way around). This explains the high volume of reports.

This is the last finding I had (despite having lost some duplicates due to this change in the process). As you can verify, they are all reproducible and, being XSS vulnerabilities, they hold an average rating of Medium Severity.
I don't mean to offend, and I hope this last report can be reopened so we can wrap things up this way :)

Thank you very much!

@Hexix23
Copy link
Copy Markdown
Author

Hexix23 commented May 6, 2026

@alan-agius4 The other bugs have been fixed. Could you take another look at the PR and check the comments? Thanks, and sorry for the trouble :)

@alan-agius4
Copy link
Copy Markdown
Contributor

alan-agius4 commented May 6, 2026

This also will be handled by #68591

@Hexix23
Copy link
Copy Markdown
Author

Hexix23 commented May 6, 2026

Thanks for confirming! So we can conclude that it was a security risk, right? @alan-agius4

@alan-agius4
Copy link
Copy Markdown
Contributor

alan-agius4 commented May 6, 2026

It’s a possibility, but the risk is pretty low since someone would have to inject the malicious content in the translations first. Still, it’s a good move for hardening the system.

This was referenced May 6, 2026
Closed
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@josephperrott josephperrott josephperrott approved these changes

@alan-agius4 alan-agius4 Awaiting requested review from alan-agius4

@thePunderWoman thePunderWoman Awaiting requested review from thePunderWoman

Assignees

No one assigned

Labels

area: compiler Issues related to `ngc`, Angular's template compiler

Projects

None yet

Milestone

Backlog

Development

Successfully merging this pull request may close these issues.

3 participants

@Hexix23 @alan-agius4 @josephperrott