From 134c8006c00f4a53579e392e4a1a1a5c60157778 Mon Sep 17 00:00:00 2001 From: Lee Kelleher Date: Tue, 8 Apr 2025 13:32:36 +0100 Subject: [PATCH] `umb-localize` encode HTML arguments (#18960) * Moves `escapeHTML` call from localization controller to `umb-localize` element * Adds supporting unit-test * Removed unit-test as it is now expected that the localization controller will return literal HTML markup. * Updated import path * Removed extra call to `text()` --- .../localization.controller.test.ts | 6 ------ .../localization-api/localization.controller.ts | 11 +++-------- .../core/localization/localize.element.test.ts | 8 ++++++++ .../packages/core/localization/localize.element.ts | 14 ++++++++++---- .../core/utils/sanitize/escape-html.function.ts | 2 +- 5 files changed, 22 insertions(+), 19 deletions(-) diff --git a/src/Umbraco.Web.UI.Client/src/libs/localization-api/localization.controller.test.ts b/src/Umbraco.Web.UI.Client/src/libs/localization-api/localization.controller.test.ts index a01f922f72..fbe5e7e890 100644 --- a/src/Umbraco.Web.UI.Client/src/libs/localization-api/localization.controller.test.ts +++ b/src/Umbraco.Web.UI.Client/src/libs/localization-api/localization.controller.test.ts @@ -175,12 +175,6 @@ describe('UmbLocalizeController', () => { expect((controller.term as any)('logout', 'Hello', 'World')).to.equal('Log out'); }); - it('should encode HTML entities', () => { - expect(controller.term('withInlineToken', 'Hello', ''), 'XSS detected').to.equal( - 'Hello <script>alert("XSS")</script>', - ); - }); - it('only reacts to changes of its own localization-keys', async () => { const element: UmbLocalizationRenderCountElement = await fixture( html``, diff --git a/src/Umbraco.Web.UI.Client/src/libs/localization-api/localization.controller.ts b/src/Umbraco.Web.UI.Client/src/libs/localization-api/localization.controller.ts index cb9e3927a5..c85be3797e 100644 --- a/src/Umbraco.Web.UI.Client/src/libs/localization-api/localization.controller.ts +++ b/src/Umbraco.Web.UI.Client/src/libs/localization-api/localization.controller.ts @@ -20,7 +20,6 @@ import type { import { umbLocalizationManager } from './localization.manager.js'; import type { LitElement } from '@umbraco-cms/backoffice/external/lit'; import type { UmbController, UmbControllerHost } from '@umbraco-cms/backoffice/controller-api'; -import { escapeHTML } from '@umbraco-cms/backoffice/utils'; const LocalizationControllerAlias = Symbol(); /** @@ -137,20 +136,16 @@ export class UmbLocalizationController escapeHTML(a)); - if (typeof term === 'function') { - return term(...sanitizedArgs) as string; + return term(...args) as string; } if (typeof term === 'string') { - if (sanitizedArgs.length) { + if (args.length) { // Replace placeholders of format "%index%" and "{index}" with provided values term = term.replace(/(%(\d+)%|\{(\d+)\})/g, (match, _p1, p2, p3): string => { const index = p2 || p3; - return typeof sanitizedArgs[index] !== 'undefined' ? String(sanitizedArgs[index]) : match; + return typeof args[index] !== 'undefined' ? String(args[index]) : match; }); } } diff --git a/src/Umbraco.Web.UI.Client/src/packages/core/localization/localize.element.test.ts b/src/Umbraco.Web.UI.Client/src/packages/core/localization/localize.element.test.ts index ae69b81fd1..66f5487a1d 100644 --- a/src/Umbraco.Web.UI.Client/src/packages/core/localization/localize.element.test.ts +++ b/src/Umbraco.Web.UI.Client/src/packages/core/localization/localize.element.test.ts @@ -95,6 +95,14 @@ describe('umb-localize', () => { expect(element.shadowRoot?.innerHTML).to.contain('Hello World'); }); + it('should localize a key with multiple arguments as encoded HTML', async () => { + element.key = 'general_moreThanOneArgument'; + element.args = ['Hello', 'World']; + await elementUpdated(element); + + expect(element.shadowRoot?.innerHTML).to.contain('<strong>Hello</strong> <em>World</em>'); + }); + it('should localize a key with args as an attribute', async () => { element.key = 'general_moreThanOneArgument'; element.setAttribute('args', '["Hello","World"]'); diff --git a/src/Umbraco.Web.UI.Client/src/packages/core/localization/localize.element.ts b/src/Umbraco.Web.UI.Client/src/packages/core/localization/localize.element.ts index 945209f072..11aa824808 100644 --- a/src/Umbraco.Web.UI.Client/src/packages/core/localization/localize.element.ts +++ b/src/Umbraco.Web.UI.Client/src/packages/core/localization/localize.element.ts @@ -1,4 +1,5 @@ import { css, customElement, html, property, state, unsafeHTML } from '@umbraco-cms/backoffice/external/lit'; +import { escapeHTML } from '@umbraco-cms/backoffice/utils'; import { UmbLitElement } from '@umbraco-cms/backoffice/lit-element'; /** @@ -34,7 +35,11 @@ export class UmbLocalizeElement extends UmbLitElement { @state() protected get text(): string { - const localizedValue = this.localize.term(this.key, ...(this.args ?? [])); + // As translated texts can contain HTML, we will need to render with unsafeHTML. + // But arguments can come from user input, so they should be escaped. + const escapedArgs = (this.args ?? []).map((a) => escapeHTML(a)); + + const localizedValue = this.localize.term(this.key, ...escapedArgs); // If the value is the same as the key, it means the key was not found. if (localizedValue === this.key) { @@ -44,12 +49,13 @@ export class UmbLocalizeElement extends UmbLitElement { (this.getHostElement() as HTMLElement).removeAttribute('data-localize-missing'); - return localizedValue; + return localizedValue.trim(); } override render() { - return this.text.trim() - ? html`${unsafeHTML(this.text)}` + const text = this.text; + return text + ? unsafeHTML(text) : this.debug ? html`${this.key}` : html``; diff --git a/src/Umbraco.Web.UI.Client/src/packages/core/utils/sanitize/escape-html.function.ts b/src/Umbraco.Web.UI.Client/src/packages/core/utils/sanitize/escape-html.function.ts index ee84b1ee86..e183b0dac2 100644 --- a/src/Umbraco.Web.UI.Client/src/packages/core/utils/sanitize/escape-html.function.ts +++ b/src/Umbraco.Web.UI.Client/src/packages/core/utils/sanitize/escape-html.function.ts @@ -4,7 +4,7 @@ const NON_ALPHANUMERIC_REGEXP = /([^#-~| |!])/g; /** * Escapes HTML entities in a string. - * @example escapeHTML(''), // "<script>alert("XSS")</script>" + * @example escapeHTML(''), // "<script>alert("XSS")</script>" * @param html The HTML string to escape. * @returns The sanitized HTML string. */