From 7d58b798c626bb0e4e5f89ca8affdce4f352b232 Mon Sep 17 00:00:00 2001 From: Doug Parker Date: Fri, 13 Feb 2026 14:59:57 -0800 Subject: [PATCH] fix(core): block creation of sensitive URI attributes from ICU messages Translators are not allowed to write HTML which creates URI attributes. I opted to ban any values going into an attribute at all, to prevent even links to malicious content, rather than just sanitizing URIs. I also converted this blocklist into an allowlist. Now, we only allowing setting known attributes (while sanitizing URI attributes). This significantly reduces risk of missing a vulnerable attribute and does not require an exhaustive list of all potential attributes. BREAKING CHANGE: Angular now only applies known attributes from HTML in translated ICU content. Unknown attributes are dropped and not rendered. (cherry picked from commit 306f367899dfc2e04238fecd3455547b5d54075d) --- packages/core/src/render3/i18n/i18n_parse.ts | 37 ++++++++++++---- packages/core/test/acceptance/i18n_spec.ts | 4 +- .../core/test/render3/i18n/i18n_parse_spec.ts | 44 +++++++++++++++++++ 3 files changed, 75 insertions(+), 10 deletions(-) diff --git a/packages/core/src/render3/i18n/i18n_parse.ts b/packages/core/src/render3/i18n/i18n_parse.ts index 5aab5ce2e14..b31aad7e2a9 100644 --- a/packages/core/src/render3/i18n/i18n_parse.ts +++ b/packages/core/src/render3/i18n/i18n_parse.ts @@ -808,7 +808,6 @@ function walkIcuTree( const attr = elAttrs.item(i)!; const lowerAttrName = attr.name.toLowerCase(); const hasBinding = !!attr.value.match(BINDING_REGEXP); - // we assume the input string is safe, unless it's using a binding if (hasBinding) { if (VALID_ATTRS.hasOwnProperty(lowerAttrName)) { if (URI_ATTRS[lowerAttrName]) { @@ -831,8 +830,29 @@ function walkIcuTree( `(see ${XSS_SECURITY_URL})`, ); } + } else if (VALID_ATTRS[lowerAttrName]) { + if (URI_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) { + console.warn( + `WARNING: ignoring unsafe attribute ` + + `${lowerAttrName} on element ${tagName} ` + + `(see ${XSS_SECURITY_URL})`, + ); + } + addCreateAttribute(create, newIndex, attr.name, 'unsafe:blocked'); + } else { + addCreateAttribute(create, newIndex, attr.name, attr.value); + } } else { - addCreateAttribute(create, newIndex, attr); + if (typeof ngDevMode !== 'undefined' && ngDevMode) { + console.warn( + `WARNING: ignoring unknown attribute name ` + + `${lowerAttrName} on element ${tagName} ` + + `(see ${XSS_SECURITY_URL})`, + ); + } } } const elementNode: I18nElementNode = { @@ -945,10 +965,11 @@ function addCreateNodeAndAppend( ); } -function addCreateAttribute(create: IcuCreateOpCodes, newIndex: number, attr: Attr) { - create.push( - (newIndex << IcuCreateOpCode.SHIFT_REF) | IcuCreateOpCode.Attr, - attr.name, - attr.value, - ); +function addCreateAttribute( + create: IcuCreateOpCodes, + newIndex: number, + attrName: string, + attrValue: string, +) { + create.push((newIndex << IcuCreateOpCode.SHIFT_REF) | IcuCreateOpCode.Attr, attrName, attrValue); } diff --git a/packages/core/test/acceptance/i18n_spec.ts b/packages/core/test/acceptance/i18n_spec.ts index 3b3babc1255..e7f8aaab19d 100644 --- a/packages/core/test/acceptance/i18n_spec.ts +++ b/packages/core/test/acceptance/i18n_spec.ts @@ -3434,7 +3434,7 @@ describe('runtime i18n', () => { {parameters.length, plural, =1 { Affects parameter - {{ parameters[0].name }} + {{ parameters[0].name }} } other { Affects {{parameters.length}} parameters, including @@ -3453,7 +3453,7 @@ describe('runtime i18n', () => { const fixture = TestBed.createComponent(MyApp); fixture.detectChanges(); const span = (fixture.nativeElement as HTMLElement).querySelector('span')!; - expect(span.getAttribute('attr')).toEqual('should_be_present'); + expect(span.getAttribute('label')).toEqual('should_be_present'); expect(span.getAttribute('class')).toEqual('parameter-name'); }); diff --git a/packages/core/test/render3/i18n/i18n_parse_spec.ts b/packages/core/test/render3/i18n/i18n_parse_spec.ts index e2bde83db26..53fb556113e 100644 --- a/packages/core/test/render3/i18n/i18n_parse_spec.ts +++ b/packages/core/test/render3/i18n/i18n_parse_spec.ts @@ -297,6 +297,50 @@ describe('i18n_parse', () => { ); }); }); + + it('should properly sanitize malicious URLs like `` injected into translations', () => { + const tI18n = toT18n(`{ + �0�, select, + A {malicious JS} + other {malicious link} + }`); + + fixture.apply(() => { + applyCreateOpCodes(fixture.lView, tI18n.create, fixture.host, null); + expect(fixture.host.innerHTML).toEqual(``); + }); + + fixture.apply(() => { + ɵɵi18nExp('A'); + ɵɵi18nApply(0); + expect(fixture.host.innerHTML).toEqual( + `malicious JS`, + ); + }); + + fixture.apply(() => { + ɵɵi18nExp('other'); + ɵɵi18nApply(0); + expect(fixture.host.innerHTML).toEqual( + `malicious link`, + ); + }); + }); + + it('should ignore unknown attributes', () => { + const tI18n = toT18n(`{�0�, select, A {
} }`); + + fixture.apply(() => { + applyCreateOpCodes(fixture.lView, tI18n.create, fixture.host, null); + expect(fixture.host.innerHTML).toEqual(``); + }); + + fixture.apply(() => { + ɵɵi18nExp('A'); + ɵɵi18nApply(0); + expect(fixture.host.innerHTML).toEqual(`
`); + }); + }); }); function toT18n(text: string) {