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

alan-agius4 · mattrbeck · commit 83a640516f7b · 2026-05-07T15:25:14.000-07:00
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 PR Close #68469
diff --git a/packages/core/src/render3/i18n/i18n_parse.ts b/packages/core/src/render3/i18n/i18n_parse.ts
@@ -996,7 +996,7 @@ function i18nSanitizeAttribute(attrName: string): SanitizerFn | null {
   }
 
   if (SECURITY_SENSITIVE_ATTRS.has(lowerAttrName)) {
-    return _validateAttribute;
+    return <SanitizerFn>_validateAttribute;
   }
 
   return null;
diff --git a/packages/core/src/render3/instructions/shared.ts b/packages/core/src/render3/instructions/shared.ts
@@ -49,6 +49,7 @@ import {
   LViewFlags,
   RENDERER,
   TData,
+  TVIEW,
   TView,
 } from '../interfaces/view';
 import {assertTNodeType} from '../node_assert';
@@ -469,14 +470,18 @@ export function elementAttributeInternal(
 ) {
   if (ngDevMode) {
     assertNotSame(value, NO_CHANGE as any, 'Incoming value should never be NO_CHANGE.');
-    validateAgainstEventAttributes(name);
     assertTNodeType(
       tNode,
       TNodeType.Element,
       `Attempted to set attribute \`${name}\` on a container node. ` +
         `Host bindings are not valid on ng-container or ng-template.`,
     );
   }
+
+  if (lView[TVIEW].firstUpdatePass) {
+    validateAgainstEventAttributes(name);
+  }
+
   const element = getNativeByTNode(tNode, lView) as RElement;
   setElementAttribute(lView[RENDERER], element, namespace, tNode.value, name, value, sanitizer);
 }
diff --git a/packages/core/src/sanitization/sanitization.ts b/packages/core/src/sanitization/sanitization.ts
@@ -265,10 +265,12 @@ export function validateAgainstEventProperties(name: string) {
 
 export function validateAgainstEventAttributes(name: string) {
   if (name.toLowerCase().startsWith('on')) {
-    const errorMessage =
-      `Binding to event attribute '${name}' is disallowed for security reasons, ` +
-      `please use (${name.slice(2)})=...`;
-    throw new RuntimeError(RuntimeErrorCode.INVALID_EVENT_BINDING, errorMessage);
+    throw new RuntimeError(
+      RuntimeErrorCode.INVALID_EVENT_BINDING,
+      ngDevMode &&
+        `Binding to event attribute '${name}' is disallowed for security reasons, ` +
+          `please use (${name.slice(2)})=...`,
+    );
   }
 }
 
@@ -283,7 +285,7 @@ const attributeName: ReadonlySet<string> = new Set(['attributename']);
  * @remarks Keep this in sync with DOM Security Schema.
  * @see [SECURITY_SCHEMA](../../../compiler/src/schema/dom_security_schema.ts)
  */
-const SECURITY_SENSITIVE_ELEMENTS: Readonly<Record<string, ReadonlySet<string>>> = {
+export const SECURITY_SENSITIVE_ELEMENTS: Readonly<Record<string, ReadonlySet<string>>> = {
   'iframe': new Set([
     'sandbox',
     'allow',
@@ -305,11 +307,7 @@ const SECURITY_SENSITIVE_ELEMENTS: Readonly<Record<string, ReadonlySet<string>>>
  * @param tagName The name of the tag.
  * @param attributeName The name of the attribute.
  */
-export function ɵɵvalidateAttribute(
-  value: unknown,
-  tagName: string,
-  attributeName: string,
-): unknown {
+export function ɵɵvalidateAttribute<T = any>(value: T, tagName: string, attributeName: string): T {
   const lowerCaseTagName = tagName.toLowerCase();
   const lowerCaseAttrName = attributeName.toLowerCase();
   if (!SECURITY_SENSITIVE_ELEMENTS[lowerCaseTagName]?.has(lowerCaseAttrName)) {
diff --git a/packages/core/test/linker/security_integration_spec.ts b/packages/core/test/linker/security_integration_spec.ts
@@ -30,16 +30,14 @@ class OnPrefixDir {
 
 describe('security integration tests', function () {
   beforeEach(() => {
+    // Disable logging for these tests.
+    spyOn(console, 'log').and.callFake(() => {});
+
     TestBed.configureTestingModule({
       declarations: [SecuredComponent, OnPrefixDir],
     });
   });
 
-  beforeEach(() => {
-    // Disable logging for these tests.
-    spyOn(console, 'log').and.callFake(() => {});
-  });
-
   describe('events', () => {
     // this test is similar to the previous one, but since on-prefixed attributes validation now
     // happens at runtime, we need to invoke change detection to trigger elementProperty call
@@ -89,6 +87,43 @@ describe('security integration tests', function () {
       expect(div.nativeElement.onclick).not.toBe(value);
       expect(div.nativeElement.hasAttribute('onclick')).toEqual(false);
     });
+
+    for (const ngDevModeValue of [true, false]) {
+      it(`should disallow binding to attr.on* in host bindings with ngDevMode=${ngDevModeValue}`, () => {
+        const originalNgDevMode = (globalThis as any).ngDevMode;
+        (globalThis as any).ngDevMode = ngDevModeValue;
+
+        @Directive({
+          selector: '[dirOnclick]',
+          standalone: false,
+        })
+        class LocalHostOnclickDirective {
+          @HostBinding('attr.onclick') @Input() dirOnclick: string | undefined;
+        }
+
+        @Component({
+          selector: 'local-comp',
+          template: `<button [dirOnclick]="ctxProp"></button>`,
+          standalone: false,
+        })
+        class LocalSecuredComponent {
+          ctxProp: any = 'some value';
+        }
+
+        try {
+          TestBed.configureTestingModule({
+            declarations: [LocalSecuredComponent, LocalHostOnclickDirective],
+          });
+
+          expect(() => {
+            const cmp = TestBed.createComponent(LocalSecuredComponent);
+            cmp.detectChanges();
+          }).toThrowError(/NG0306/);
+        } finally {
+          (globalThis as any).ngDevMode = originalNgDevMode;
+        }
+      });
+    }
   });
 
   describe('safe HTML values', function () {

Original file line number	Diff line number	Diff line change
`@@ -996,7 +996,7 @@ function i18nSanitizeAttribute(attrName: string): SanitizerFn \| null {`
`996`	`996`	`}`
`997`	`997`
`998`	`998`	`if (SECURITY_SENSITIVE_ATTRS.has(lowerAttrName)) {`
`999`		`- return _validateAttribute;`
	`999`	`+ return <SanitizerFn>_validateAttribute;`
`1000`	`1000`	`}`
`1001`	`1001`
`1002`	`1002`	`return null;`