Merge commit from fork

AndyButland · iOvergaard · web-flow · commit d4f8754f9338 · 2025-01-20T14:43:27.000+01:00
* Escape HTML in strings provided as arguments for client-side localization.

* Used HTML sanitizer from dependency.

* Moved sanitization to controller.

* Fixed path to dependency.

* fix: move constants to their own respective files to avoid circular dependencies

* fix: import through import map

* fix: import `sanitizeHTML` from importmap

* chore: consts

* feat: add more ops/sec to `term` function

* fix: check specifically for `typeof 'undefined'` before determining if an arg is valid or not

in case where the sanitizer removes everything so the arg is just an empty string (`""`) we still want it to replace the token, i.e. with the empty string

* test: add tests to verify localization controller xss

* chore: try to improve ops/sec and readability of the `term` function

* test: add more tests for the sanitizer

* fix(dictionary workspace): rely on localization sanitization

* feat: adds new function to escape html entities

* fix: use `escapeHTML` to escape/encode only HTML entities

* test: update test to reflect correct escape

* test: add tests for sanitizeHTML and escapeHTML

* fix: update function to not escape all quotes as people may want to keep those

* chore: formatting

---------

Co-authored-by: Jacob Overgaard &lt;752371+iOvergaard@users.noreply.github.com&gt;
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
@@ -175,6 +175,12 @@ describe('UmbLocalizeController', () => {
 			expect((controller.term as any)('logout', 'Hello', 'World')).to.equal('Log out');
 		});
 
+		it('should encode HTML entities', () => {
+			expect(controller.term('withInlineToken', 'Hello', '<script>alert("XSS")</script>'), 'XSS detected').to.equal(
+				'Hello &lt;script&gt;alert(&#34;XSS&#34;)&lt;/script&gt;',
+			);
+		});
+
 		it('only reacts to changes of its own localization-keys', async () => {
 			const element: UmbLocalizationRenderCountElement = await fixture(
 				html`<umb-localization-render-count></umb-localization-render-count>`,
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
@@ -20,6 +20,7 @@ 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();
 /**
@@ -119,29 +120,35 @@ export class UmbLocalizationController<LocalizationSetType extends UmbLocalizati
 		}
 
 		const { primary, secondary } = this.getLocalizationData(this.lang());
+
+		// eslint-disable-next-line @typescript-eslint/no-explicit-any
 		let term: any;
 
 		// Look for a matching term using regionCode, code, then the fallback
-		if (primary && primary[key]) {
+		if (primary?.[key]) {
 			term = primary[key];
-		} else if (secondary && secondary[key]) {
+		} else if (secondary?.[key]) {
 			term = secondary[key];
-		} else if (umbLocalizationManager.fallback && umbLocalizationManager.fallback[key]) {
+		} else if (umbLocalizationManager.fallback?.[key]) {
 			term = umbLocalizationManager.fallback[key];
 		} else {
 			return String(key);
 		}
 
+		// 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 sanitizedArgs = args.map((a) => escapeHTML(a));
+
 		if (typeof term === 'function') {
-			return term(...args) as string;
+			return term(...sanitizedArgs) as string;
 		}
 
 		if (typeof term === 'string') {
-			if (args.length > 0) {
+			if (sanitizedArgs.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 String(args[index] || match);
+					return typeof sanitizedArgs[index] !== 'undefined' ? String(sanitizedArgs[index]) : match;
 				});
 			}
 		}
diff --git a/src/Umbraco.Web.UI.Client/src/packages/core/auth/auth-flow.ts b/src/Umbraco.Web.UI.Client/src/packages/core/auth/auth-flow.ts
@@ -13,7 +13,7 @@
  * License for the specific language governing permissions and limitations under
  * the License.
  */
-import { UMB_STORAGE_TOKEN_RESPONSE_NAME } from './auth.context.token.js';
+import { UMB_STORAGE_TOKEN_RESPONSE_NAME } from './constants.js';
 import type { LocationLike, StringMap } from '@umbraco-cms/backoffice/external/openid';
 import {
 	BaseTokenRequestHandler,
diff --git a/src/Umbraco.Web.UI.Client/src/packages/core/auth/auth.context.token.ts b/src/Umbraco.Web.UI.Client/src/packages/core/auth/auth.context.token.ts
@@ -2,5 +2,3 @@ import type { UmbAuthContext } from './auth.context.js';
 import { UmbContextToken } from '@umbraco-cms/backoffice/context-api';
 
 export const UMB_AUTH_CONTEXT = new UmbContextToken<UmbAuthContext>('UmbAuthContext');
-export const UMB_STORAGE_TOKEN_RESPONSE_NAME = 'umb:userAuthTokenResponse';
-export const UMB_STORAGE_REDIRECT_URL = 'umb:auth:redirect';
diff --git a/src/Umbraco.Web.UI.Client/src/packages/core/auth/auth.context.ts b/src/Umbraco.Web.UI.Client/src/packages/core/auth/auth.context.ts
@@ -1,7 +1,8 @@
 import { UmbAuthFlow } from './auth-flow.js';
-import { UMB_AUTH_CONTEXT, UMB_STORAGE_TOKEN_RESPONSE_NAME } from './auth.context.token.js';
+import { UMB_AUTH_CONTEXT } from './auth.context.token.js';
 import type { UmbOpenApiConfiguration } from './models/openApiConfiguration.js';
 import type { ManifestAuthProvider } from './auth-provider.extension.js';
+import { UMB_STORAGE_TOKEN_RESPONSE_NAME } from './constants.js';
 import { OpenAPI } from '@umbraco-cms/backoffice/external/backend-api';
 import type { UmbControllerHost } from '@umbraco-cms/backoffice/controller-api';
 import { UmbContextBase } from '@umbraco-cms/backoffice/class-api';
diff --git a/src/Umbraco.Web.UI.Client/src/packages/core/auth/components/auth-provider-default.element.ts b/src/Umbraco.Web.UI.Client/src/packages/core/auth/components/auth-provider-default.element.ts
@@ -1,15 +1,17 @@
 import type { UmbAuthProviderDefaultProps, UmbUserLoginState } from '../types.js';
-import { UmbLitElement } from '../../lit-element/lit-element.element.js';
-import { UmbTextStyles } from '../../style/index.js';
 import type { ManifestAuthProvider } from '../auth-provider.extension.js';
 import { css, customElement, html, nothing, property } from '@umbraco-cms/backoffice/external/lit';
+import { UmbLitElement } from '@umbraco-cms/backoffice/lit-element';
+import { UmbTextStyles } from '@umbraco-cms/backoffice/style';
 
 @customElement('umb-auth-provider-default')
 export class UmbAuthProviderDefaultElement extends UmbLitElement implements UmbAuthProviderDefaultProps {
 	@property({ attribute: false })
 	userLoginState?: UmbUserLoginState | undefined;
+
 	@property({ attribute: false })
 	manifest!: ManifestAuthProvider;
+
 	@property({ attribute: false })
 	onSubmit!: (manifestOrProviderName: string | ManifestAuthProvider, loginHint?: string) => void;
 
diff --git a/src/Umbraco.Web.UI.Client/src/packages/core/auth/constants.ts b/src/Umbraco.Web.UI.Client/src/packages/core/auth/constants.ts
@@ -0,0 +1 @@
+export const UMB_STORAGE_TOKEN_RESPONSE_NAME = 'umb:userAuthTokenResponse';
diff --git a/src/Umbraco.Web.UI.Client/src/packages/core/auth/index.ts b/src/Umbraco.Web.UI.Client/src/packages/core/auth/index.ts
@@ -2,6 +2,7 @@ import './components/index.js';
 
 export * from './auth.context.js';
 export * from './auth.context.token.js';
+export * from './constants.js';
 export * from './modals/index.js';
 export type * from './models/openApiConfiguration.js';
 export * from './components/index.js';
diff --git a/src/Umbraco.Web.UI.Client/src/packages/core/utils/index.ts b/src/Umbraco.Web.UI.Client/src/packages/core/utils/index.ts
@@ -19,6 +19,7 @@ export * from './path/stored-path.function.js';
 export * from './path/transform-server-path-to-client-path.function.js';
 export * from './path/umbraco-path.function.js';
 export * from './path/url-pattern-to-string.function.js';
+export * from './sanitize/escape-html.function.js';
 export * from './sanitize/sanitize-html.function.js';
 export * from './selection-manager/selection.manager.js';
 export * from './state-manager/index.js';
diff --git a/src/Umbraco.Web.UI.Client/src/packages/core/utils/path/stored-path.function.test.ts b/src/Umbraco.Web.UI.Client/src/packages/core/utils/path/stored-path.function.test.ts
@@ -1,6 +1,5 @@
-import { retrieveStoredPath, setStoredPath } from './stored-path.function.js';
+import { retrieveStoredPath, setStoredPath, UMB_STORAGE_REDIRECT_URL } from './stored-path.function.js';
 import { expect } from '@open-wc/testing';
-import { UMB_STORAGE_REDIRECT_URL } from '@umbraco-cms/backoffice/auth';
 
 describe('retrieveStoredPath', () => {
 	beforeEach(() => {
diff --git a/src/Umbraco.Web.UI.Client/src/packages/core/utils/path/stored-path.function.ts b/src/Umbraco.Web.UI.Client/src/packages/core/utils/path/stored-path.function.ts
@@ -1,5 +1,6 @@
 import { ensureLocalPath } from './ensure-local-path.function.js';
-import { UMB_STORAGE_REDIRECT_URL } from '@umbraco-cms/backoffice/auth';
+
+export const UMB_STORAGE_REDIRECT_URL = 'umb:auth:redirect';
 
 /**
  * Retrieve the stored path from the session storage.
diff --git a/src/Umbraco.Web.UI.Client/src/packages/core/utils/sanitize/escape-html.function.test.ts b/src/Umbraco.Web.UI.Client/src/packages/core/utils/sanitize/escape-html.function.test.ts
@@ -0,0 +1,8 @@
+import { expect } from '@open-wc/testing';
+import { escapeHTML } from './escape-html.function.js';
+
+describe('escapeHtml', () => {
+	it('should escape html', () => {
+		expect(escapeHTML('<script>alert("XSS")</script>')).to.equal('&lt;script&gt;alert(&#34;XSS&#34;)&lt;/script&gt;');
+	});
+});
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
@@ -0,0 +1,29 @@
+const SURROGATE_PAIR_REGEXP = /[\uD800-\uDBFF][\uDC00-\uDFFF]/g;
+// Match everything outside of normal chars and " (quote character)
+const NON_ALPHANUMERIC_REGEXP = /([^#-~| |!])/g;
+
+/**
+ * Escapes HTML entities in a string.
+ * @example escapeHTML('<script>alert("XSS")</script>'), // "&lt;script&gt;alert(&quot;XSS&quot;)&lt;/script&gt;"
+ * @param html The HTML string to escape.
+ * @returns The sanitized HTML string.
+ */
+export function escapeHTML(html: unknown): string {
+	if (typeof html !== 'string' && html instanceof String === false) {
+		return html as string;
+	}
+
+	return html
+		.toString()
+		.replace(/&/g, '&amp;')
+		.replace(SURROGATE_PAIR_REGEXP, function (value) {
+			const hi = value.charCodeAt(0);
+			const low = value.charCodeAt(1);
+			return '&#' + ((hi - 0xd800) * 0x400 + (low - 0xdc00) + 0x10000) + ';';
+		})
+		.replace(NON_ALPHANUMERIC_REGEXP, function (value) {
+			return '&#' + value.charCodeAt(0) + ';';
+		})
+		.replace(/</g, '&lt;')
+		.replace(/>/g, '&gt;');
+}
diff --git a/src/Umbraco.Web.UI.Client/src/packages/core/utils/sanitize/sanitize-html.function.test.ts b/src/Umbraco.Web.UI.Client/src/packages/core/utils/sanitize/sanitize-html.function.test.ts
@@ -0,0 +1,24 @@
+import { expect } from '@open-wc/testing';
+import { sanitizeHTML } from './sanitize-html.function.js';
+
+describe('sanitizeHTML', () => {
+	it('should allow benign HTML', () => {
+		expect(sanitizeHTML('<strong>Test</strong>')).to.equal('<strong>Test</strong>');
+	});
+
+	it('should remove potentially harmful content', () => {
+		expect(sanitizeHTML('<script>alert("XSS")</script>')).to.equal('');
+	});
+
+	it('should remove potentially harmful attributes', () => {
+		expect(sanitizeHTML('<a href="javascript:alert(\'XSS\')">Test</a>')).to.equal('<a>Test</a>');
+	});
+
+	it('should remove potentially harmful content and attributes', () => {
+		expect(sanitizeHTML('<a href="javascript:alert(\'XSS\')"><script>alert("XSS")</script></a>')).to.equal('<a></a>');
+	});
+
+	it('should allow benign attributes', () => {
+		expect(sanitizeHTML('<a href="/test">Test</a>')).to.equal('<a href="/test">Test</a>');
+	});
+});
diff --git a/src/Umbraco.Web.UI.Client/src/packages/dictionary/workspace/views/workspace-view-dictionary-editor.element.ts b/src/Umbraco.Web.UI.Client/src/packages/dictionary/workspace/views/workspace-view-dictionary-editor.element.ts
@@ -6,7 +6,6 @@ import { css, html, customElement, state, repeat } from '@umbraco-cms/backoffice
 import { UmbLitElement } from '@umbraco-cms/backoffice/lit-element';
 import { UmbLanguageCollectionRepository, type UmbLanguageDetailModel } from '@umbraco-cms/backoffice/language';
 import { UMB_CURRENT_USER_CONTEXT } from '@umbraco-cms/backoffice/current-user';
-import { sanitizeHTML } from '@umbraco-cms/backoffice/utils';
 
 @customElement('umb-workspace-view-dictionary-editor')
 export class UmbWorkspaceViewDictionaryEditorElement extends UmbLitElement {
@@ -22,10 +21,6 @@ export class UmbWorkspaceViewDictionaryEditorElement extends UmbLitElement {
 	@state()
 	private _currentUserHasAccessToAllLanguages?: boolean = false;
 
-	get #dictionaryName() {
-		return typeof this._dictionary?.name !== 'undefined' ? sanitizeHTML(this._dictionary.name) : '...';
-	}
-
 	readonly #languageCollectionRepository = new UmbLanguageCollectionRepository(this);
 	#workspaceContext?: typeof UMB_DICTIONARY_WORKSPACE_CONTEXT.TYPE;
 	#currentUserContext?: typeof UMB_CURRENT_USER_CONTEXT.TYPE;
@@ -89,7 +84,7 @@ export class UmbWorkspaceViewDictionaryEditorElement extends UmbLitElement {
 	override render() {
 		return html`
 			<uui-box>
-				${this.localize.term('dictionaryItem_description', this.#dictionaryName)}
+				<umb-localize key="dictionaryItem_description" .args=${[this._dictionary?.name ?? '...']}></umb-localize>
 				${repeat(
 					this._languages,
 					(item) => item.unique,
diff --git a/src/Umbraco.Web.UI.Client/utils/all-umb-consts/index.ts b/src/Umbraco.Web.UI.Client/utils/all-umb-consts/index.ts
@@ -52,7 +52,7 @@ export const foundConsts = [{
 			},
 {
 				path: '@umbraco-cms/backoffice/auth',
-				consts: ["UMB_AUTH_CONTEXT","UMB_STORAGE_TOKEN_RESPONSE_NAME","UMB_STORAGE_REDIRECT_URL","UMB_MODAL_APP_AUTH"]
+				consts: ["UMB_AUTH_CONTEXT","UMB_STORAGE_TOKEN_RESPONSE_NAME","UMB_MODAL_APP_AUTH"]
 			},
 {
 				path: '@umbraco-cms/backoffice/block-custom-view',
@@ -400,7 +400,7 @@ export const foundConsts = [{
 			},
 {
 				path: '@umbraco-cms/backoffice/utils',
-				consts: []
+				consts: ["UMB_STORAGE_REDIRECT_URL"]
 			},
 {
 				path: '@umbraco-cms/backoffice/validation',

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1 @@`
	`1`	`+export const UMB_STORAGE_TOKEN_RESPONSE_NAME = 'umb:userAuthTokenResponse';`