fix(editor): Fixing XSS vulnerability in toast messages (#10329)

Co-authored-by: Adi <aditya@netroy.in>
n8n-io · Aug 8, 2024 · 38bdd9f · 38bdd9f
1 parent b6c47c0
commit 38bdd9f
Show file tree

Hide file tree

Showing 5 changed files with 26 additions and 19 deletions.
diff --git a/packages/editor-ui/src/components/NpsSurvey.vue b/packages/editor-ui/src/components/NpsSurvey.vue
@@ -94,6 +94,7 @@ async function send() {
 			message: Number(form.value.value) >= 8 ? i18n.baseText('prompts.npsSurvey.reviewUs') : '',
 			type: 'success',
 			duration: 15000,
+			dangerouslyUseHTMLString: true,
 		});
 
 		setTimeout(() => {

diff --git a/packages/editor-ui/src/composables/useExecutionDebugging.ts b/packages/editor-ui/src/composables/useExecutionDebugging.ts
@@ -16,6 +16,7 @@ import { useUIStore } from '@/stores/ui.store';
 import { useTelemetry } from './useTelemetry';
 import { useRootStore } from '@/stores/root.store';
 import { isFullExecutionResponse } from '@/utils/typeGuards';
+import { sanitizeHtml } from '@/utils/htmlUtils';
 
 export const useExecutionDebugging = () => {
 	const telemetry = useTelemetry();
@@ -61,7 +62,7 @@ export const useExecutionDebugging = () => {
 				h(
 					'ul',
 					{ class: 'mt-l ml-l' },
-					matchingPinnedNodeNames.map((name) => h('li', name)),
+					matchingPinnedNodeNames.map((name) => h('li', sanitizeHtml(name))),
 				),
 			]);
 

diff --git a/packages/editor-ui/src/composables/usePushConnection.ts b/packages/editor-ui/src/composables/usePushConnection.ts
@@ -328,6 +328,7 @@ export function usePushConnection({ router }: { router: ReturnType<typeof useRou
 					message: `${action} <a href="https://docs.n8n.io/integrations/builtin/core-nodes/n8n-nodes-base.wait/" target="_blank">More info</a>`,
 					type: 'success',
 					duration: 0,
+					dangerouslyUseHTMLString: true,
 				});
 			} else if (runDataExecuted.finished !== true) {
 				titleChange.titleSet(workflow.name as string, 'ERROR');
@@ -438,7 +439,6 @@ export function usePushConnection({ router }: { router: ReturnType<typeof useRou
 							message: runDataExecutedErrorMessage,
 							type: 'error',
 							duration: 0,
-							dangerouslyUseHTMLString: true,
 						});
 					}
 				}

diff --git a/packages/editor-ui/src/composables/useToast.ts b/packages/editor-ui/src/composables/useToast.ts
@@ -18,7 +18,7 @@ export interface NotificationErrorWithNodeAndDescription extends ApplicationErro
 }
 
 const messageDefaults: Partial<Omit<NotificationOptions, 'message'>> = {
-	dangerouslyUseHTMLString: true,
+	dangerouslyUseHTMLString: false,
 	position: 'bottom-right',
 };
 
@@ -32,28 +32,28 @@ export function useToast() {
 	const i18n = useI18n();
 
 	function showMessage(messageData: Partial<NotificationOptions>, track = true) {
-		messageData = { ...messageDefaults, ...messageData };
-
-		Object.defineProperty(messageData, 'message', {
-			value:
-				typeof messageData.message === 'string'
-					? sanitizeHtml(messageData.message)
-					: messageData.message,
-			writable: true,
-			enumerable: true,
-		});
+		const { message, title } = messageData;
+		const params = { ...messageDefaults, ...messageData };
+
+		if (typeof message === 'string') {
+			params.message = sanitizeHtml(message);
+		}
+
+		if (typeof title === 'string') {
+			params.title = sanitizeHtml(title);
+		}
 
-		const notification = Notification(messageData);
+		const notification = Notification(params);
 
-		if (messageData.duration === 0) {
+		if (params.duration === 0) {
 			stickyNotificationQueue.push(notification);
 		}
 
-		if (messageData.type === 'error' && track) {
+		if (params.type === 'error' && track) {
 			telemetry.track('Instance FE emitted error', {
-				error_title: messageData.title,
-				error_message: messageData.message,
-				caused_by_credential: causedByCredential(messageData.message as string),
+				error_title: params.title,
+				error_message: params.message,
+				caused_by_credential: causedByCredential(params.message as string),
 				workflow_id: workflowsStore.workflowId,
 			});
 		}
@@ -133,6 +133,7 @@ export function useToast() {
 					${collapsableDetails(error)}`,
 				type: 'error',
 				duration: 0,
+				dangerouslyUseHTMLString: true,
 			},
 			false,
 		);

diff --git a/packages/editor-ui/src/utils/htmlUtils.ts b/packages/editor-ui/src/utils/htmlUtils.ts
@@ -18,6 +18,10 @@ export function sanitizeHtml(dirtyHtml: string) {
 			}
 
 			if (ALLOWED_HTML_ATTRIBUTES.includes(name) || name.startsWith('data-')) {
+				// href is allowed but we need to sanitize certain protocols
+				if (name === 'href' && !value.match(/^https?:\/\//gm)) {
+					return '';
+				}
 				return `${name}="${friendlyAttrValue(value)}"`;
 			}