Skip to content

fix(compiler): register SVG animation attributes in URL security context#67797

Closed
alan-agius4 wants to merge 6 commits intoangular:mainfrom
alan-agius4:security-hardning-animation
Closed

fix(compiler): register SVG animation attributes in URL security context#67797
alan-agius4 wants to merge 6 commits intoangular:mainfrom
alan-agius4:security-hardning-animation

Conversation

@alan-agius4
Copy link
Copy Markdown
Contributor

@alan-agius4 alan-agius4 commented Mar 23, 2026

See individual commit messages.

@alan-agius4 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 Mar 23, 2026
@angular-robot angular-robot bot added the area: compiler Issues related to `ngc`, Angular's template compiler label Mar 23, 2026
@angular-robot angular-robot bot added the area: core Issues related to the framework runtime label Mar 23, 2026
@ngbot ngbot bot added this to the Backlog milestone Mar 23, 2026
@alan-agius4 alan-agius4 force-pushed the security-hardning-animation branch 5 times, most recently from 9dcbdec to caa4cdb Compare March 23, 2026 13:48
@alan-agius4 alan-agius4 force-pushed the security-hardning-animation branch 2 times, most recently from 2e56c83 to fb05d71 Compare March 24, 2026 09:06
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

Copy link
Copy Markdown
Member

@pkozlowski-opensource pkozlowski-opensource 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

This change is a security hardening measure to prevent potentially unsafe attribute value manipulation through SVG animations. By mapping `animate|to`, `animate|from`, `animate|values`, and `set|to` to the `SecurityContext.URL`,  Angular will now automatically sanitize these attributes.
Previously, the `data` attribute of the `<object>` tag was being sanitized as a regular URL instead of a `ResourceURL`, which is security-sensitive.
This commit updates the runtime sanitization logic to correctly identify `object[data]` as a `ResourceURL` context. Additionally, the sanitizer lookup logic has been refactored to use a more efficient lookup map (`RESOURCE_MAP`) instead of multiple `Set` lookups, providing better performance and maintainability.

Added tests to verify the correct sanitization of `object[data]` and its behavior with trusted values.
SVG animation elements (`animate` and `set`) can be used to animate sensitive attributes like `href` or `xlink:href`. Binding to these animation attributes (like `to`, `from`, or `values`) with a sensitive target creates an XSS vector.

This change mitigates this risk by:
1. Classifying `to`, `from`, and `values` on `<animate>` and `<set>` elements as `ATTRIBUTE_NO_BINDING` in the DOM security schema to prevent standard dynamic bindings.
2. Adding runtime validations in `ɵɵvalidateAttribute` to verify that `attributeName` is not a sensitive attribute (such as `href` or `xlink:href`) when processed by a set of `SECURITY_SENSITIVE_ATTRIBUTE_NAMES`. If it is, a runtime error `UNSAFE_ATTRIBUTE_BINDING` is thrown.
3. Adding regression tests in `integration_spec.ts` to ensure unsafe bindings throw an error while safe ones pass correctly.
@alan-agius4 alan-agius4 force-pushed the security-hardning-animation branch from 94beafe to e2b740c Compare March 30, 2026 17:08
@AndrewKushnir AndrewKushnir removed the action: review The PR is still awaiting reviews from at least one requested reviewer label Mar 30, 2026
@alan-agius4 alan-agius4 added action: merge The PR is ready for merge by the caretaker merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note and removed action: merge The PR is ready for merge by the caretaker labels Mar 30, 2026
@alan-agius4
Copy link
Copy Markdown
Contributor Author

alan-agius4 commented Mar 31, 2026

caretaker note: the g3 failure is pre-existing

@pkozlowski-opensource
Copy link
Copy Markdown
Member

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

pkozlowski-opensource pushed a commit that referenced this pull request Apr 1, 2026
Previously, the `data` attribute of the `<object>` tag was being sanitized as a regular URL instead of a `ResourceURL`, which is security-sensitive.
This commit updates the runtime sanitization logic to correctly identify `object[data]` as a `ResourceURL` context. Additionally, the sanitizer lookup logic has been refactored to use a more efficient lookup map (`RESOURCE_MAP`) instead of multiple `Set` lookups, providing better performance and maintainability.

Added tests to verify the correct sanitization of `object[data]` and its behavior with trusted values.

PR Close #67797
pkozlowski-opensource pushed a commit that referenced this pull request Apr 1, 2026
#67797)

SVG animation elements (`animate` and `set`) can be used to animate sensitive attributes like `href` or `xlink:href`. Binding to these animation attributes (like `to`, `from`, or `values`) with a sensitive target creates an XSS vector.

This change mitigates this risk by:
1. Classifying `to`, `from`, and `values` on `<animate>` and `<set>` elements as `ATTRIBUTE_NO_BINDING` in the DOM security schema to prevent standard dynamic bindings.
2. Adding runtime validations in `ɵɵvalidateAttribute` to verify that `attributeName` is not a sensitive attribute (such as `href` or `xlink:href`) when processed by a set of `SECURITY_SENSITIVE_ATTRIBUTE_NAMES`. If it is, a runtime error `UNSAFE_ATTRIBUTE_BINDING` is thrown.
3. Adding regression tests in `integration_spec.ts` to ensure unsafe bindings throw an error while safe ones pass correctly.

PR Close #67797
pkozlowski-opensource pushed a commit that referenced this pull request Apr 1, 2026
…ext (#67797)

This change is a security hardening measure to prevent potentially unsafe attribute value manipulation through SVG animations. By mapping `animate|to`, `animate|from`, `animate|values`, and `set|to` to the `SecurityContext.URL`,  Angular will now automatically sanitize these attributes.

PR Close #67797
pkozlowski-opensource pushed a commit that referenced this pull request Apr 1, 2026
Previously, the `data` attribute of the `<object>` tag was being sanitized as a regular URL instead of a `ResourceURL`, which is security-sensitive.
This commit updates the runtime sanitization logic to correctly identify `object[data]` as a `ResourceURL` context. Additionally, the sanitizer lookup logic has been refactored to use a more efficient lookup map (`RESOURCE_MAP`) instead of multiple `Set` lookups, providing better performance and maintainability.

Added tests to verify the correct sanitization of `object[data]` and its behavior with trusted values.

PR Close #67797
pkozlowski-opensource pushed a commit that referenced this pull request Apr 1, 2026
#67797)

SVG animation elements (`animate` and `set`) can be used to animate sensitive attributes like `href` or `xlink:href`. Binding to these animation attributes (like `to`, `from`, or `values`) with a sensitive target creates an XSS vector.

This change mitigates this risk by:
1. Classifying `to`, `from`, and `values` on `<animate>` and `<set>` elements as `ATTRIBUTE_NO_BINDING` in the DOM security schema to prevent standard dynamic bindings.
2. Adding runtime validations in `ɵɵvalidateAttribute` to verify that `attributeName` is not a sensitive attribute (such as `href` or `xlink:href`) when processed by a set of `SECURITY_SENSITIVE_ATTRIBUTE_NAMES`. If it is, a runtime error `UNSAFE_ATTRIBUTE_BINDING` is thrown.
3. Adding regression tests in `integration_spec.ts` to ensure unsafe bindings throw an error while safe ones pass correctly.

PR Close #67797
@alan-agius4 alan-agius4 deleted the security-hardning-animation branch April 1, 2026 09:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

action: merge The PR is ready for merge by the caretaker area: compiler Issues related to `ngc`, Angular's template compiler area: core Issues related to the framework runtime merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note target: patch This PR is targeted for the next patch release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants