Skip to content

fix(core): disallow event attribute bindings in host bindings unconditionally#68438

Merged
mattrbeck merged 4 commits into
angular:mainfrom
alan-agius4:security-clicks
May 7, 2026
Merged

fix(core): disallow event attribute bindings in host bindings unconditionally#68438
mattrbeck merged 4 commits into
angular:mainfrom
alan-agius4:security-clicks

Conversation

@alan-agius4

Copy link
Copy Markdown
Contributor

Moves the event attribute validation check outside of ngDevMode in the elementAttributeInternal instruction to ensure that bindings to event attributes like on* are always blocked at runtime.

Previously, this check was only performed when ngDevMode was true, which could allow attacker-controlled CMS data to be bound to event attributes in production mode, causing browser-executed XSS.

Fixes #68419

@angular-robot angular-robot Bot added the area: core Issues related to the framework runtime label Apr 29, 2026
@ngbot ngbot Bot added this to the Backlog milestone Apr 29, 2026
…tionally

Moves the event attribute validation check outside of `ngDevMode` in the `elementAttributeInternal` instruction to ensure that bindings to event attributes like `on*` are always blocked at runtime.

Previously, this check was only performed when `ngDevMode` was `true`, which could allow attacker-controlled CMS data to be bound to event attributes in production mode, causing browser-executed XSS.

Fixes angular#68419
@alan-agius4 alan-agius4 requested a review from atscott April 29, 2026 08:37
@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 Apr 29, 2026
@alan-agius4 alan-agius4 marked this pull request as ready for review April 29, 2026 08:37
@pullapprove pullapprove Bot requested a review from josephperrott April 29, 2026 08:37
Comment thread packages/core/src/render3/instructions/shared.ts Outdated

@josephperrott josephperrott left a comment

Copy link
Copy Markdown
Member

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

Comment thread packages/core/src/render3/instructions/shared.ts Outdated
@alan-agius4 alan-agius4 removed the request for review from atscott April 29, 2026 16:20
@alan-agius4 alan-agius4 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 Apr 29, 2026
@alan-agius4 alan-agius4 requested a review from alxhub April 30, 2026 06:37
@AndrewKushnir AndrewKushnir removed the request for review from alxhub April 30, 2026 15:02
@AndrewKushnir AndrewKushnir added action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews and removed action: merge The PR is ready for merge by the caretaker labels Apr 30, 2026
@alan-agius4 alan-agius4 force-pushed the security-clicks branch 2 times, most recently from 620c35a to bd6fccf Compare May 4, 2026 09:19
@alan-agius4 alan-agius4 requested a review from alxhub May 4, 2026 09:22
@alan-agius4 alan-agius4 removed the action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews label May 4, 2026
@alan-agius4 alan-agius4 added the action: review The PR is still awaiting reviews from at least one requested reviewer label May 4, 2026

@alxhub alxhub left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Reviewed-for: fw-security

@alan-agius4 alan-agius4 added action: merge The PR is ready for merge by the caretaker action: presubmit The PR is in need of a google3 presubmit and removed action: review The PR is still awaiting reviews from at least one requested reviewer action: presubmit The PR is in need of a google3 presubmit labels May 6, 2026
@mattrbeck mattrbeck merged commit 5b421c6 into angular:main May 7, 2026
30 of 31 checks passed
@mattrbeck

Copy link
Copy Markdown
Member

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

@angular-automatic-lock-bot

Copy link
Copy Markdown

This pull request has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot Bot locked and limited conversation to collaborators Jun 7, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

action: merge The PR is ready for merge by the caretaker area: core Issues related to the framework runtime target: patch This PR is targeted for the next patch release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Security: Angular production mode allows [attr.onclick] and host attr.onclick bindings to become click-triggered XSS

6 participants