fix(editor): Prevent clipboard xss injection (n8n-io#10894)

thepandaguitar · Sep 20, 2024 · e20ab59 · e20ab59
1 parent 48294e7
commit e20ab59
Show file tree

Hide file tree

Showing 4 changed files with 18 additions and 18 deletions.
diff --git a/packages/editor-ui/src/composables/__tests__/useClipboard.test.ts b/packages/editor-ui/src/composables/__tests__/useClipboard.test.ts
@@ -1,4 +1,4 @@
-import { render, within } from '@testing-library/vue';
+import { render } from '@testing-library/vue';
 import userEvent from '@testing-library/user-event';
 import { defineComponent, h, ref } from 'vue';
 import { useClipboard } from '@/composables/useClipboard';
@@ -8,13 +8,9 @@ const testValue = 'This is a test';
 const TestComponent = defineComponent({
 	setup() {
 		const pasted = ref('');
-		const htmlContent = ref<HTMLElement>();
 		const clipboard = useClipboard({
 			onPaste(data) {
 				pasted.value = data;
-				if (htmlContent.value) {
-					htmlContent.value.innerHTML = data;
-				}
 			},
 		});
 
@@ -27,7 +23,6 @@ const TestComponent = defineComponent({
 					},
 				}),
 				h('div', { 'data-test-id': 'paste' }, pasted.value),
-				h('div', { 'data-test-id': 'xss-attack', ref: htmlContent }),
 			]);
 	},
 });
@@ -73,12 +68,4 @@ describe('useClipboard()', () => {
 			expect(pasteElement.textContent).toEqual(testValue);
 		});
 	});
-
-	it('sanitizes HTML', async () => {
-		const unsafeHtml = 'https://www.ex.com/sfefdfd<img/src/onerror=alert(1)>fdf/xdfef.json';
-		const { getByTestId } = render(TestComponent);
-
-		await userEvent.paste(unsafeHtml);
-		expect(within(getByTestId('xss-attack')).queryByRole('img')).not.toBeInTheDocument();
-	});
 });
diff --git a/packages/editor-ui/src/composables/useClipboard.ts b/packages/editor-ui/src/composables/useClipboard.ts
@@ -1,7 +1,6 @@
 import { onBeforeUnmount, onMounted, ref } from 'vue';
 import { useClipboard as useClipboardCore } from '@vueuse/core';
 import { useDebounce } from '@/composables/useDebounce';
-import { sanitizeIfString } from '@/utils/htmlUtils';
 
 type ClipboardEventFn = (data: string, event?: ClipboardEvent) => void;
 
@@ -43,7 +42,7 @@ export function useClipboard(
 
 		const clipboardData = event.clipboardData;
 		if (clipboardData !== null) {
-			const clipboardValue = sanitizeIfString(clipboardData.getData('text/plain'));
+			const clipboardValue = clipboardData.getData('text/plain');
 			onPasteCallback.value(clipboardValue, event);
 		}
 	}

diff --git a/packages/editor-ui/src/utils/__tests__/htmlUtils.spec.ts b/packages/editor-ui/src/utils/__tests__/htmlUtils.spec.ts
@@ -37,4 +37,18 @@ describe('sanitizeHtml', () => {
 		const result = sanitizeHtml(dirtyHtml);
 		expect(result).toBe('<a>Click me</a>');
 	});
+
+	test.each([
+		[
+			'https://www.ex.com/sfefdfd<img/src/onerror=alert(1)>fdf/xdfef.json',
+			'https://www.ex.com/sfefdfdfdf/xdfef.json',
+		],
+		[
+			// eslint-disable-next-line n8n-local-rules/no-unneeded-backticks
+			`https://www.ex.com/sfefdfd<details title='"><img/src/onerror=alert(document.domain)>/ '>/c.json`,
+			'https://www.ex.com/sfefdfd<details title="&quot;&gt;&lt;img/src/onerror=alert(document.domain)&gt;/">/c.json',
+		],
+	])('should escape js code %s to equal %s', (dirtyURL, expected) => {
+		expect(sanitizeHtml(dirtyURL)).toBe(expected);
+	});
 });
diff --git a/packages/editor-ui/src/utils/htmlUtils.ts b/packages/editor-ui/src/utils/htmlUtils.ts
@@ -1,4 +1,4 @@
-import xss, { friendlyAttrValue } from 'xss';
+import xss, { escapeAttrValue } from 'xss';
 import { ALLOWED_HTML_ATTRIBUTES, ALLOWED_HTML_TAGS } from '@/constants';
 
 /*
@@ -22,7 +22,7 @@ export function sanitizeHtml(dirtyHtml: string) {
 				if (name === 'href' && !value.match(/^https?:\/\//gm)) {
 					return '';
 				}
-				return `${name}="${friendlyAttrValue(value)}"`;
+				return `${name}="${escapeAttrValue(value)}"`;
 			}
 
 			return;