diff --git a/packages/core/src/execution-engine/node-execution-context/__tests__/__snapshots__/html-sandbox.test.ts.snap b/packages/core/src/execution-engine/node-execution-context/__tests__/__snapshots__/html-sandbox.test.ts.snap new file mode 100644 index 0000000000..ae20790253 --- /dev/null +++ b/packages/core/src/execution-engine/node-execution-context/__tests__/__snapshots__/html-sandbox.test.ts.snap @@ -0,0 +1,25 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`sandboxHtmlResponse should handle HTML with special characters 1`] = ` +" + " +`; + +exports[`sandboxHtmlResponse should handle empty HTML 1`] = ` +" + " +`; + +exports[`sandboxHtmlResponse should replace ampersands and double quotes in HTML 1`] = ` +" + " +`; diff --git a/packages/core/src/execution-engine/node-execution-context/__tests__/html-sandbox.test.ts b/packages/core/src/execution-engine/node-execution-context/__tests__/html-sandbox.test.ts new file mode 100644 index 0000000000..b33a7b2196 --- /dev/null +++ b/packages/core/src/execution-engine/node-execution-context/__tests__/html-sandbox.test.ts @@ -0,0 +1,72 @@ +import { isHtmlRenderedContentType, sandboxHtmlResponse } from '@/html-sandbox'; + +describe('sandboxHtmlResponse', () => { + it('should replace ampersands and double quotes in HTML', () => { + const html = '
Content & more
'; + expect(sandboxHtmlResponse(html)).toMatchSnapshot(); + }); + + it('should handle empty HTML', () => { + const html = ''; + expect(sandboxHtmlResponse(html)).toMatchSnapshot(); + }); + + it('should handle HTML with special characters', () => { + const html = '

Special characters: <>&"\'

'; + expect(sandboxHtmlResponse(html)).toMatchSnapshot(); + }); +}); + +describe('isHtmlRenderedContentType', () => { + it('should return true for text/html content type', () => { + const contentType = 'text/html'; + expect(isHtmlRenderedContentType(contentType)).toBe(true); + }); + + it('should return true for application/xhtml+xml content type', () => { + const contentType = 'application/xhtml+xml'; + expect(isHtmlRenderedContentType(contentType)).toBe(true); + }); + + it('should return false for other content types', () => { + const contentType = 'application/json'; + expect(isHtmlRenderedContentType(contentType)).toBe(false); + }); + + describe('should handle various HTML content types', () => { + test.each([ + 'text/html', + 'TEXT/HTML', + 'text/html; charset=utf-8', + 'text/html; charset=iso-8859-1', + 'application/xhtml+xml', + 'APPLICATION/XHTML+XML', + 'application/xhtml+xml; charset=utf-8', + ])('should return true for %s', (contentType) => { + expect(isHtmlRenderedContentType(contentType)).toBe(true); + }); + }); + + describe('should handle non-HTML content types', () => { + test.each([ + 'text/plain', + 'application/xml', + 'text/css', + 'application/javascript', + 'image/png', + 'application/pdf', + '', + 'html', + 'xhtml', + ])('should return false for %s', (contentType) => { + expect(isHtmlRenderedContentType(contentType)).toBe(false); + }); + }); + + it('should handle edge cases', () => { + expect(isHtmlRenderedContentType('text/htmlsomething')).toBe(true); + expect(isHtmlRenderedContentType('application/xhtml+xmlsomething')).toBe(true); + expect(isHtmlRenderedContentType(' text/html')).toBe(false); + expect(isHtmlRenderedContentType('text/html ')).toBe(true); + }); +}); diff --git a/packages/core/src/html-sandbox.ts b/packages/core/src/html-sandbox.ts new file mode 100644 index 0000000000..5581cb307e --- /dev/null +++ b/packages/core/src/html-sandbox.ts @@ -0,0 +1,28 @@ +/** + * Sandboxes the HTML response to prevent possible exploitation. Embeds the + * response in an iframe to make sure the HTML has a different origin. + */ +export const sandboxHtmlResponse = (html: string) => { + // Escape & and " as mentioned in the spec: + // https://html.spec.whatwg.org/multipage/iframe-embed-object.html#the-iframe-element + const escapedHtml = html.replaceAll('&', '&').replaceAll('"', '"'); + + return ` + `; +}; + +/** + * Checks if the given content type is something a browser might render + * as HTML. + */ +export const isHtmlRenderedContentType = (contentType: string) => { + const contentTypeLower = contentType.toLowerCase(); + + return ( + // The content-type can also contain a charset, e.g. "text/html; charset=utf-8" + contentTypeLower.startsWith('text/html') || contentTypeLower.startsWith('application/xhtml+xml') + ); +}; diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 19ff684dd0..660a90e09b 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -7,6 +7,7 @@ export * from './data-deduplication-service'; export * from './encryption'; export * from './errors'; export * from './execution-engine'; +export * from './html-sandbox'; export * from './instance-settings'; export * from './nodes-loader'; export * from './utils'; diff --git a/packages/nodes-base/nodes/RespondToWebhook/RespondToWebhook.node.ts b/packages/nodes-base/nodes/RespondToWebhook/RespondToWebhook.node.ts index 4de9575ea0..5618478d65 100644 --- a/packages/nodes-base/nodes/RespondToWebhook/RespondToWebhook.node.ts +++ b/packages/nodes-base/nodes/RespondToWebhook/RespondToWebhook.node.ts @@ -1,5 +1,6 @@ import jwt from 'jsonwebtoken'; import set from 'lodash/set'; +import { isHtmlRenderedContentType, sandboxHtmlResponse } from 'n8n-core'; import type { IDataObject, IExecuteFunctions, @@ -12,7 +13,6 @@ import type { } from 'n8n-workflow'; import { jsonParse, - BINARY_ENCODING, NodeOperationError, NodeConnectionTypes, WEBHOOK_NODE_TYPE, @@ -22,8 +22,9 @@ import { } from 'n8n-workflow'; import type { Readable } from 'stream'; -import { configuredOutputs } from './utils'; import { formatPrivateKey, generatePairedItemData } from '../../utils/utilities'; +import { configuredOutputs } from './utils/outputs'; +import { getBinaryResponse } from './utils/binary'; const respondWithProperty: INodeProperties = { displayName: 'Respond With', @@ -359,6 +360,9 @@ export class RespondToWebhook implements INodeType { } } + const hasHtmlContentType = + headers['content-type'] && isHtmlRenderedContentType(headers['content-type'] as string); + let statusCode = (options.responseCode as number) || 200; let responseBody: IN8nHttpResponse | Readable; if (respondWith === 'json') { @@ -412,7 +416,12 @@ export class RespondToWebhook implements INodeType { ? set({}, options.responseKey as string, items[0].json) : items[0].json; } else if (respondWith === 'text') { - responseBody = this.getNodeParameter('responseBody', 0) as string; + // If a user doesn't set the content-type header and uses html, the html can still be rendered on the browser + if (hasHtmlContentType || !headers['content-type']) { + responseBody = sandboxHtmlResponse(this.getNodeParameter('responseBody', 0) as string); + } else { + responseBody = this.getNodeParameter('responseBody', 0) as string; + } } else if (respondWith === 'binary') { const item = items[0]; @@ -438,16 +447,8 @@ export class RespondToWebhook implements INodeType { } const binaryData = this.helpers.assertBinaryData(0, responseBinaryPropertyName); - if (binaryData.id) { - responseBody = { binaryData }; - } else { - responseBody = Buffer.from(binaryData.data, BINARY_ENCODING); - headers['content-length'] = (responseBody as Buffer).length; - } - if (!headers['content-type']) { - headers['content-type'] = binaryData.mimeType; - } + responseBody = getBinaryResponse(binaryData, headers); } else if (respondWith === 'redirect') { headers.location = this.getNodeParameter('redirectURL', 0) as string; statusCode = (options.responseCode as number) ?? 307; @@ -458,6 +459,15 @@ export class RespondToWebhook implements INodeType { ); } + if ( + hasHtmlContentType && + respondWith !== 'text' && + respondWith !== 'binary' && + responseBody + ) { + responseBody = sandboxHtmlResponse(JSON.stringify(responseBody as string)); + } + response = { body: responseBody, headers, diff --git a/packages/nodes-base/nodes/RespondToWebhook/test/RespondToWebhook.test.ts b/packages/nodes-base/nodes/RespondToWebhook/test/RespondToWebhook.test.ts index c2bece0a99..f1380eb05d 100644 --- a/packages/nodes-base/nodes/RespondToWebhook/test/RespondToWebhook.test.ts +++ b/packages/nodes-base/nodes/RespondToWebhook/test/RespondToWebhook.test.ts @@ -1,6 +1,6 @@ import type { DeepMockProxy } from 'jest-mock-extended'; import { mock, mockDeep } from 'jest-mock-extended'; -import { constructExecutionMetaData } from 'n8n-core'; +import { constructExecutionMetaData, sandboxHtmlResponse } from 'n8n-core'; import { BINARY_ENCODING, WAIT_NODE_TYPE, @@ -163,7 +163,7 @@ describe('RespondToWebhook Node', () => { await expect(respondToWebhook.execute.call(mockExecuteFunctions)).resolves.not.toThrow(); expect(mockExecuteFunctions.sendResponse).toHaveBeenCalledWith({ - body: 'responseBody', + body: sandboxHtmlResponse('responseBody'), headers: {}, statusCode: 200, }); @@ -263,6 +263,74 @@ describe('RespondToWebhook Node', () => { expect(mockExecuteFunctions.sendResponse).not.toHaveBeenCalled(); }); + describe('HTML content sandboxing', () => { + it('should sandbox HTML content for json response with HTML content-type', async () => { + const inputItems = [ + { json: { index: 0, input: true } }, + { json: { index: 1, input: true } }, + ]; + mockExecuteFunctions.getInputData.mockReturnValue(inputItems); + mockExecuteFunctions.getNode.mockReturnValue(mock({ typeVersion: 1.1 })); + mockExecuteFunctions.getParentNodes.mockReturnValue([ + mock({ type: WAIT_NODE_TYPE }), + ]); + mockExecuteFunctions.getNodeParameter.mockImplementation((paramName) => { + if (paramName === 'respondWith') return 'allIncomingItems'; + if (paramName === 'options') + return { + responseHeaders: { + entries: [{ name: 'content-type', value: 'application/xhtml+xml' }], + }, + }; + }); + mockExecuteFunctions.sendResponse.mockReturnValue(); + + const result = await respondToWebhook.execute.call(mockExecuteFunctions); + expect(mockExecuteFunctions.sendResponse).toHaveBeenCalledWith({ + body: sandboxHtmlResponse(JSON.stringify(inputItems.map((item) => item.json))), + headers: { 'content-type': 'application/xhtml+xml' }, + statusCode: 200, + }); + expect(result).toHaveLength(1); + expect(result[0]).toHaveLength(2); + expect(result[0]).toEqual(inputItems); + }); + + it('should NOT sandbox HTML content for non-HTML content-type', async () => { + const inputItems = [ + { json: { index: 0, input: true } }, + { json: { index: 1, input: true } }, + ]; + mockExecuteFunctions.getInputData.mockReturnValue(inputItems); + mockExecuteFunctions.getNode.mockReturnValue(mock({ typeVersion: 1.1 })); + mockExecuteFunctions.getParentNodes.mockReturnValue([ + mock({ type: WAIT_NODE_TYPE }), + ]); + mockExecuteFunctions.getNodeParameter.mockImplementation((paramName) => { + if (paramName === 'respondWith') return 'allIncomingItems'; + if (paramName === 'options') return {}; + }); + mockExecuteFunctions.sendResponse.mockReturnValue(); + + const result = await respondToWebhook.execute.call(mockExecuteFunctions); + expect(mockExecuteFunctions.sendResponse).toHaveBeenCalledWith({ + body: inputItems.map((item) => item.json), + headers: {}, + statusCode: 200, + }); + expect(result).toHaveLength(1); + expect(result[0]).toHaveLength(2); + expect(result[0]).toEqual(inputItems); + + await expect(respondToWebhook.execute.call(mockExecuteFunctions)).resolves.not.toThrow(); + expect(mockExecuteFunctions.sendResponse).toHaveBeenCalledWith({ + body: inputItems.map((item) => item.json), + headers: {}, + statusCode: 200, + }); + }); + }); + it('should have two outputs in version 1.3', async () => { const inputItems = [{ json: { index: 0, input: true } }, { json: { index: 1, input: true } }]; mockExecuteFunctions.getInputData.mockReturnValue(inputItems); diff --git a/packages/nodes-base/nodes/RespondToWebhook/test/binary.test.ts b/packages/nodes-base/nodes/RespondToWebhook/test/binary.test.ts new file mode 100644 index 0000000000..a770fae19c --- /dev/null +++ b/packages/nodes-base/nodes/RespondToWebhook/test/binary.test.ts @@ -0,0 +1,65 @@ +import { sandboxHtmlResponse } from 'n8n-core'; +import type { IDataObject } from 'n8n-workflow'; +import { BINARY_ENCODING } from 'n8n-workflow'; + +import { getBinaryResponse } from '../utils/binary'; + +describe('getBinaryResponse', () => { + it('returns sanitized HTML when binaryData.id is present and mimeType is text/html', () => { + const binaryData = { + id: '123', + data: '

Hello

', + mimeType: 'text/html', + }; + const headers: IDataObject = {}; + + const result = getBinaryResponse(binaryData, headers); + + expect(result).toBe(sandboxHtmlResponse(binaryData.data)); + expect(headers['content-type']).toBe('text/html'); + }); + + it('returns { binaryData } when binaryData.id is present and mimeType is not text/html', () => { + const binaryData = { + id: '123', + data: 'some-binary-data', + mimeType: 'application/octet-stream', + }; + const headers: IDataObject = {}; + + const result = getBinaryResponse(binaryData, headers); + + expect(result).toEqual({ binaryData }); + expect(headers['content-type']).toBe('application/octet-stream'); + }); + + it('returns sanitized HTML when binaryData.id is not present and mimeType is text/html', () => { + const binaryData = { + data: '

Hello

', + mimeType: 'text/html', + }; + const headers: IDataObject = {}; + + const result = getBinaryResponse(binaryData, headers); + + expect(result).toBe( + sandboxHtmlResponse(Buffer.from(binaryData.data, BINARY_ENCODING).toString()), + ); + expect(headers['content-type']).toBe('text/html'); + }); + + it('returns Buffer when binaryData.id is not present and mimeType is not text/html', () => { + const binaryData = { + data: 'some-binary-data', + mimeType: 'application/octet-stream', + }; + const headers: IDataObject = {}; + + const result = getBinaryResponse(binaryData, headers); + + expect(Buffer.isBuffer(result)).toBe(true); + expect(result.toString()).toBe(Buffer.from(binaryData.data, BINARY_ENCODING).toString()); + expect(headers['content-type']).toBe('application/octet-stream'); + expect(headers['content-length']).toBe(Buffer.from(binaryData.data, BINARY_ENCODING).length); + }); +}); diff --git a/packages/nodes-base/nodes/RespondToWebhook/test/utils.test.ts b/packages/nodes-base/nodes/RespondToWebhook/test/outputs.test.ts similarity index 95% rename from packages/nodes-base/nodes/RespondToWebhook/test/utils.test.ts rename to packages/nodes-base/nodes/RespondToWebhook/test/outputs.test.ts index 497d29ebe5..7a023d39be 100644 --- a/packages/nodes-base/nodes/RespondToWebhook/test/utils.test.ts +++ b/packages/nodes-base/nodes/RespondToWebhook/test/outputs.test.ts @@ -1,4 +1,4 @@ -import { configuredOutputs } from '../utils'; +import { configuredOutputs } from '../utils/outputs'; describe('configuredOutputs', () => { it('returns array of objects when version >= 1.3', () => { diff --git a/packages/nodes-base/nodes/RespondToWebhook/utils/binary.ts b/packages/nodes-base/nodes/RespondToWebhook/utils/binary.ts new file mode 100644 index 0000000000..5da549c667 --- /dev/null +++ b/packages/nodes-base/nodes/RespondToWebhook/utils/binary.ts @@ -0,0 +1,42 @@ +import { isHtmlRenderedContentType, sandboxHtmlResponse } from 'n8n-core'; +import type { IBinaryData, IDataObject, IN8nHttpResponse } from 'n8n-workflow'; +import { BINARY_ENCODING } from 'n8n-workflow'; +import type { Readable } from 'stream'; + +const setContentLength = (responseBody: IN8nHttpResponse | Readable, headers: IDataObject) => { + if (Buffer.isBuffer(responseBody)) { + headers['content-length'] = responseBody.length; + } else if (typeof responseBody === 'string') { + headers['content-length'] = Buffer.byteLength(responseBody, 'utf8'); + } +}; + +/** + * Returns a response body for a binary data and sets the content-type header. + */ +export const getBinaryResponse = (binaryData: IBinaryData, headers: IDataObject) => { + const contentType = headers['content-type'] as string; + const shouldSandboxResponseData = + isHtmlRenderedContentType(binaryData.mimeType) || + (contentType && isHtmlRenderedContentType(contentType)); + + let responseBody: IN8nHttpResponse | Readable; + + if (binaryData.id) { + responseBody = shouldSandboxResponseData + ? sandboxHtmlResponse(binaryData.data) + : { binaryData }; + } else { + const responseBuffer = Buffer.from(binaryData.data, BINARY_ENCODING); + + responseBody = shouldSandboxResponseData + ? sandboxHtmlResponse(responseBuffer.toString()) + : responseBuffer; + + setContentLength(responseBody, headers); + } + + headers['content-type'] ??= binaryData.mimeType; + + return responseBody; +}; diff --git a/packages/nodes-base/nodes/RespondToWebhook/utils.ts b/packages/nodes-base/nodes/RespondToWebhook/utils/outputs.ts similarity index 100% rename from packages/nodes-base/nodes/RespondToWebhook/utils.ts rename to packages/nodes-base/nodes/RespondToWebhook/utils/outputs.ts