From 94f6b03cf6840ec7a77c81578e9191cb9d880a80 Mon Sep 17 00:00:00 2001 From: Jacob Overgaard <752371+iOvergaard@users.noreply.github.com> Date: Wed, 8 Jan 2025 15:47:04 +0100 Subject: [PATCH] fix: escape HTML entities in localization arguments cherry-picked from V15 --- .../localization.controller.test.ts | 12 ++++++ .../localization.controller.ts | 37 ++++++++++++++----- src/packages/core/auth/auth-flow.ts | 2 +- src/packages/core/auth/auth.context.token.ts | 2 - src/packages/core/auth/auth.context.ts | 3 +- src/packages/core/auth/constants.ts | 1 + src/packages/core/auth/index.ts | 1 + src/packages/core/utils/index.ts | 1 + .../utils/path/stored-path.function.test.ts | 3 +- .../core/utils/path/stored-path.function.ts | 3 +- .../sanitize/escape-html.function.test.ts | 16 ++++++++ .../utils/sanitize/escape-html.function.ts | 19 ++++++++++ .../sanitize/sanitize-html.function.test.ts | 24 ++++++++++++ ...orkspace-view-dictionary-editor.element.ts | 7 +--- 14 files changed, 109 insertions(+), 22 deletions(-) create mode 100644 src/packages/core/auth/constants.ts create mode 100644 src/packages/core/utils/sanitize/escape-html.function.test.ts create mode 100644 src/packages/core/utils/sanitize/escape-html.function.ts create mode 100644 src/packages/core/utils/sanitize/sanitize-html.function.test.ts diff --git a/src/libs/localization-api/localization.controller.test.ts b/src/libs/localization-api/localization.controller.test.ts index 4126febed5..fa48a795f6 100644 --- a/src/libs/localization-api/localization.controller.test.ts +++ b/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', ''), '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``, @@ -290,6 +296,12 @@ describe('UmbLocalizeController', () => { const str = '#missing_translation_key'; expect(controller.string(str)).to.equal('#missing_translation_key'); }); + + it('should return an empty string if the input is not a string', async () => { + expect(controller.string(123)).to.equal(''); + expect(controller.string({})).to.equal(''); + expect(controller.string(undefined)).to.equal(''); + }); }); describe('host element', () => { diff --git a/src/libs/localization-api/localization.controller.ts b/src/libs/localization-api/localization.controller.ts index 49f337f3b4..1c92265e91 100644 --- a/src/libs/localization-api/localization.controller.ts +++ b/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(); /** @@ -109,8 +110,9 @@ export class UmbLocalizationController(key: K, ...args: FunctionParams): string { if (!this.#usedKeys.includes(key)) { @@ -118,29 +120,35 @@ export class UmbLocalizationController 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; }); } } @@ -178,7 +186,18 @@ export class UmbLocalizationController('UmbAuthContext'); -export const UMB_STORAGE_TOKEN_RESPONSE_NAME = 'umb:userAuthTokenResponse'; -export const UMB_STORAGE_REDIRECT_URL = 'umb:auth:redirect'; diff --git a/src/packages/core/auth/auth.context.ts b/src/packages/core/auth/auth.context.ts index 01263f311d..9d77d52d90 100644 --- a/src/packages/core/auth/auth.context.ts +++ b/src/packages/core/auth/auth.context.ts @@ -1,6 +1,7 @@ import type { UmbBackofficeExtensionRegistry, ManifestAuthProvider } from '../extension-registry/index.js'; 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 { UMB_STORAGE_TOKEN_RESPONSE_NAME } from './constants.js'; import type { UmbOpenApiConfiguration } from './models/openApiConfiguration.js'; import { OpenAPI } from '@umbraco-cms/backoffice/external/backend-api'; import type { UmbControllerHost } from '@umbraco-cms/backoffice/controller-api'; diff --git a/src/packages/core/auth/constants.ts b/src/packages/core/auth/constants.ts new file mode 100644 index 0000000000..10e5bdbb49 --- /dev/null +++ b/src/packages/core/auth/constants.ts @@ -0,0 +1 @@ +export const UMB_STORAGE_TOKEN_RESPONSE_NAME = 'umb:userAuthTokenResponse'; diff --git a/src/packages/core/auth/index.ts b/src/packages/core/auth/index.ts index 2f5cf6b67d..8aa2e1dc2c 100644 --- a/src/packages/core/auth/index.ts +++ b/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 * from './models/openApiConfiguration.js'; export * from './components/index.js'; diff --git a/src/packages/core/utils/index.ts b/src/packages/core/utils/index.ts index f19dbaeb6f..e7e6259b35 100644 --- a/src/packages/core/utils/index.ts +++ b/src/packages/core/utils/index.ts @@ -17,6 +17,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/packages/core/utils/path/stored-path.function.test.ts b/src/packages/core/utils/path/stored-path.function.test.ts index 8fc9ef5ece..409f492bf2 100644 --- a/src/packages/core/utils/path/stored-path.function.test.ts +++ b/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/packages/core/utils/path/stored-path.function.ts b/src/packages/core/utils/path/stored-path.function.ts index 01e877a5bb..e9196ab324 100644 --- a/src/packages/core/utils/path/stored-path.function.ts +++ b/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/packages/core/utils/sanitize/escape-html.function.test.ts b/src/packages/core/utils/sanitize/escape-html.function.test.ts new file mode 100644 index 0000000000..555045300f --- /dev/null +++ b/src/packages/core/utils/sanitize/escape-html.function.test.ts @@ -0,0 +1,16 @@ +import { expect } from '@open-wc/testing'; +import { escapeHTML } from './escape-html.function.js'; + +describe('escapeHtml', () => { + it('should escape html', () => { + expect(escapeHTML('')).to.equal('<script>alert("XSS")</script>'); + }); + + it('should escape html with single quotes', () => { + expect(escapeHTML("")).to.equal('<script>alert('XSS')</script>'); + }); + + it('should escape html with mixed quotes', () => { + expect(escapeHTML("")).to.equal('<script>alert('XSS')</script>'); + }); +}); diff --git a/src/packages/core/utils/sanitize/escape-html.function.ts b/src/packages/core/utils/sanitize/escape-html.function.ts new file mode 100644 index 0000000000..4ff14aac25 --- /dev/null +++ b/src/packages/core/utils/sanitize/escape-html.function.ts @@ -0,0 +1,19 @@ +/** + * Escapes HTML entities in a string. + * @example escapeHTML(''), // "<script>alert("XSS")</script>" + * @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, '&') + .replace(/"/g, '"') + .replace(/'/g, ''') + .replace(//g, '>'); +} diff --git a/src/packages/core/utils/sanitize/sanitize-html.function.test.ts b/src/packages/core/utils/sanitize/sanitize-html.function.test.ts new file mode 100644 index 0000000000..05daf44862 --- /dev/null +++ b/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('Test')).to.equal('Test'); + }); + + it('should remove potentially harmful content', () => { + expect(sanitizeHTML('')).to.equal(''); + }); + + it('should remove potentially harmful attributes', () => { + expect(sanitizeHTML('Test')).to.equal('Test'); + }); + + it('should remove potentially harmful content and attributes', () => { + expect(sanitizeHTML('')).to.equal(''); + }); + + it('should allow benign attributes', () => { + expect(sanitizeHTML('Test')).to.equal('Test'); + }); +}); diff --git a/src/packages/dictionary/workspace/views/workspace-view-dictionary-editor.element.ts b/src/packages/dictionary/workspace/views/workspace-view-dictionary-editor.element.ts index fdeaf2130b..b987fc3a64 100644 --- a/src/packages/dictionary/workspace/views/workspace-view-dictionary-editor.element.ts +++ b/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` - ${this.localize.term('dictionaryItem_description', this.#dictionaryName)} + ${repeat( this._languages, (item) => item.unique,