fix(core): sanitize translated form attributes

crisbeto · mattrbeck · commit 621c7071adff · 2026-03-12T12:44:18.000-06:00
Fixes that we weren't sanitizing the `form` and `formaction` attributes when they're used together with translations.
diff --git a/packages/core/src/render3/i18n/i18n_parse.ts b/packages/core/src/render3/i18n/i18n_parse.ts
@@ -11,7 +11,7 @@ import '../../util/ng_i18n_closure_mode';
 import {XSS_SECURITY_URL} from '../../error_details_base_url';
 import {
   getTemplateContent,
-  URI_ATTRS,
+  SENSITIVE_ATTRS,
   VALID_ATTRS,
   VALID_ELEMENTS,
 } from '../../sanitization/html_sanitizer';
@@ -388,7 +388,7 @@ export function i18nAttributesFirstPass(tView: TView, index: number, values: str
           previousElementIndex,
           attrName,
           countBindings(updateOpCodes),
-          URI_ATTRS[attrName.toLowerCase()] ? _sanitizeUrl : null,
+          SENSITIVE_ATTRS[attrName.toLowerCase()] ? _sanitizeUrl : null,
         );
       }
     }
@@ -816,7 +816,7 @@ function walkIcuTree(
                   newIndex,
                   attr.name,
                   0,
-                  URI_ATTRS[lowerAttrName] ? _sanitizeUrl : null,
+                  SENSITIVE_ATTRS[lowerAttrName] ? _sanitizeUrl : null,
                 );
               } else {
                 ngDevMode &&
@@ -827,7 +827,7 @@ function walkIcuTree(
                   );
               }
             } else if (VALID_ATTRS[lowerAttrName]) {
-              if (URI_ATTRS[lowerAttrName]) {
+              if (SENSITIVE_ATTRS[lowerAttrName]) {
                 // Don't sanitize, because no value is acceptable in sensitive attributes.
                 // Translators are not allowed to create URIs.
                 if (typeof ngDevMode !== 'undefined' && ngDevMode) {
diff --git a/packages/core/src/sanitization/html_sanitizer.ts b/packages/core/src/sanitization/html_sanitizer.ts
@@ -13,14 +13,16 @@ import {trustedHTMLFromString} from '../util/security/trusted_types';
 import {getInertBodyHelper, InertBodyHelper} from './inert_body';
 import {_sanitizeUrl} from './url_sanitizer';
 
-function tagSet(tags: string): {[k: string]: boolean} {
-  const res: {[k: string]: boolean} = {};
+type BooleanRecord = Record<string, boolean>;
+
+function tagSet(tags: string): BooleanRecord {
+  const res: BooleanRecord = {};
   for (const t of tags.split(',')) res[t] = true;
   return res;
 }
 
-function merge(...sets: {[k: string]: boolean}[]): {[k: string]: boolean} {
-  const res: {[k: string]: boolean} = {};
+function merge(...sets: BooleanRecord[]): BooleanRecord {
+  const res: BooleanRecord = {};
   for (const s of sets) {
     for (const v in s) {
       if (s.hasOwnProperty(v)) res[v] = true;
@@ -66,15 +68,15 @@ const INLINE_ELEMENTS = merge(
   ),
 );
 
-export const VALID_ELEMENTS: {[k: string]: boolean} = merge(
+export const VALID_ELEMENTS: BooleanRecord = merge(
   VOID_ELEMENTS,
   BLOCK_ELEMENTS,
   INLINE_ELEMENTS,
   OPTIONAL_END_TAG_ELEMENTS,
 );
 
 // Attributes that have href and hence need to be sanitized
-export const URI_ATTRS: {[k: string]: boolean} = tagSet(
+const URI_ATTRS: BooleanRecord = tagSet(
   'background,cite,href,itemtype,longdesc,poster,src,xlink:href',
 );
 
@@ -105,7 +107,7 @@ const ARIA_ATTRS = tagSet(
 // can be sanitized, but they increase security surface area without a legitimate use case, so they
 // are left out here.
 
-export const VALID_ATTRS: {[k: string]: boolean} = merge(URI_ATTRS, HTML_ATTRS, ARIA_ATTRS);
+export const VALID_ATTRS: BooleanRecord = merge(URI_ATTRS, HTML_ATTRS, ARIA_ATTRS);
 
 // Elements whose content should not be traversed/preserved, if the elements themselves are invalid.
 //
@@ -114,6 +116,16 @@ export const VALID_ATTRS: {[k: string]: boolean} = merge(URI_ATTRS, HTML_ATTRS,
 // don't want to preserve the content, if the elements themselves are going to be removed.
 const SKIP_TRAVERSING_CONTENT_IF_INVALID_ELEMENTS = tagSet('script,style,template');
 
+/**
+ * Attributes that are potential attach vectors and may need to be sanitized.
+ */
+export const SENSITIVE_ATTRS: BooleanRecord = merge(
+  URI_ATTRS,
+  // Note: we don't include these attributes in `URI_ATTRS`, because `URI_ATTRS` also
+  // determines whether an attribute should be dropped when sanitizing an HTML string.
+  tagSet('action,formaction,data,codebase'),
+);
+
 /**
  * SanitizingHtmlSerializer serializes a DOM fragment, stripping out any unsafe elements and unsafe
  * attributes.
diff --git a/packages/core/test/acceptance/i18n_spec.ts b/packages/core/test/acceptance/i18n_spec.ts
@@ -13,6 +13,7 @@ import {CommonModule, DOCUMENT, registerLocaleData} from '@angular/common';
 import localeEs from '@angular/common/locales/es';
 import localeRo from '@angular/common/locales/ro';
 import {computeMsgId} from '@angular/compiler';
+import {isBrowser} from '@angular/private/testing';
 import {
   Attribute,
   Component,
@@ -3669,6 +3670,27 @@ describe('runtime i18n', () => {
       expect(link).toBeTruthy();
       expect(link.getAttribute('href')).toMatch(/^unsafe:/);
     });
+
+    it('should sanitize action binding', () => {
+      const fixture = initWithTemplate(
+        SanitizeAppComp,
+        '<form action="{{url}}" i18n-action></form>',
+      );
+      const form: HTMLFormElement = fixture.nativeElement.querySelector('form');
+      expect(form.getAttribute('action')).toMatch(/^unsafe:/);
+    });
+
+    // Skip this test in Node, because Domino doesn't support `formAction`.
+    if (isBrowser) {
+      it('should sanitize formaction binding', () => {
+        const fixture = initWithTemplate(
+          SanitizeAppComp,
+          '<input type="text" formaction="{{url}}" i18n-formaction>',
+        );
+        const input: HTMLInputElement = fixture.nativeElement.querySelector('input');
+        expect(input.getAttribute('formaction')).toMatch(/^unsafe:/);
+      });
+    }
   });
 });
 
diff --git a/packages/core/test/acceptance/security_spec.ts b/packages/core/test/acceptance/security_spec.ts
@@ -803,3 +803,19 @@ describe('SVG animation processing', () => {
     );
   });
 });
+
+describe('innerHTML processing', () => {
+  it('should drop risky attributes from elements created with innerHTML', () => {
+    @Component({
+      template: '<div [innerHTML]="html"></div>',
+    })
+    class App {
+      html = '<div action="abc"></div>';
+    }
+
+    const fixture = TestBed.createComponent(App);
+    fixture.detectChanges();
+
+    expect(fixture.nativeElement.innerHTML).not.toContain('action');
+  });
+});
