From eceee7f3f8899d200b1c5720087cc494eec22e6a Mon Sep 17 00:00:00 2001 From: Tomi Turtiainen <10324676+tomi@users.noreply.github.com> Date: Wed, 15 Jan 2025 14:27:23 +0200 Subject: [PATCH] fix(core): Prevent prototype pollution of internal classes in task runner (#12610) --- packages/@n8n/task-runner/package.json | 4 +-- .../__tests__/js-task-runner.test.ts | 25 ++++++++++++++++- .../src/js-task-runner/js-task-runner.ts | 27 ++++++++++++------- pnpm-lock.yaml | 6 ++--- 4 files changed, 46 insertions(+), 16 deletions(-) diff --git a/packages/@n8n/task-runner/package.json b/packages/@n8n/task-runner/package.json index c32ea2714e..d4034b0453 100644 --- a/packages/@n8n/task-runner/package.json +++ b/packages/@n8n/task-runner/package.json @@ -40,13 +40,13 @@ "acorn": "8.14.0", "acorn-walk": "8.3.4", "lodash": "catalog:", + "luxon": "catalog:", "n8n-core": "workspace:*", "n8n-workflow": "workspace:*", "nanoid": "catalog:", "ws": "^8.18.0" }, "devDependencies": { - "@types/lodash": "catalog:", - "luxon": "catalog:" + "@types/lodash": "catalog:" } } diff --git a/packages/@n8n/task-runner/src/js-task-runner/__tests__/js-task-runner.test.ts b/packages/@n8n/task-runner/src/js-task-runner/__tests__/js-task-runner.test.ts index 6828f23cb1..a666240098 100644 --- a/packages/@n8n/task-runner/src/js-task-runner/__tests__/js-task-runner.test.ts +++ b/packages/@n8n/task-runner/src/js-task-runner/__tests__/js-task-runner.test.ts @@ -1,4 +1,4 @@ -import { DateTime } from 'luxon'; +import { DateTime, Duration, Interval } from 'luxon'; import type { IBinaryData } from 'n8n-workflow'; import { setGlobalState, type CodeExecutionMode, type IDataObject } from 'n8n-workflow'; import fs from 'node:fs'; @@ -1412,5 +1412,28 @@ describe('JsTaskRunner', () => { expect(outcome.result).toEqual([wrapIntoJson({ prototypeChanged: false })]); checkPrototypeIntact(); }); + + test('should freeze luxon prototypes', async () => { + const outcome = await executeForAllItems({ + code: ` + [DateTime, Interval, Duration] + .forEach(constructor => { + constructor.prototype.maliciousKey = 'value'; + }); + + return [] + `, + inputItems: [{ a: 1 }], + }); + + expect(outcome.result).toEqual([]); + + // @ts-expect-error Non-existing property + expect(DateTime.now().maliciousKey).toBeUndefined(); + // @ts-expect-error Non-existing property + expect(Interval.fromISO('P1Y2M10DT2H30M').maliciousKey).toBeUndefined(); + // @ts-expect-error Non-existing property + expect(Duration.fromObject({ hours: 1 }).maliciousKey).toBeUndefined(); + }); }); }); diff --git a/packages/@n8n/task-runner/src/js-task-runner/js-task-runner.ts b/packages/@n8n/task-runner/src/js-task-runner/js-task-runner.ts index 46ee1a6e7b..44745a850c 100644 --- a/packages/@n8n/task-runner/src/js-task-runner/js-task-runner.ts +++ b/packages/@n8n/task-runner/src/js-task-runner/js-task-runner.ts @@ -1,6 +1,7 @@ import set from 'lodash/set'; +import { DateTime, Duration, Interval } from 'luxon'; import { getAdditionalKeys } from 'n8n-core'; -import { WorkflowDataProxy, Workflow, ObservableObject } from 'n8n-workflow'; +import { WorkflowDataProxy, Workflow, ObservableObject, Expression } from 'n8n-workflow'; import type { CodeExecutionMode, IWorkflowExecuteAdditionalData, @@ -108,15 +109,21 @@ export class JsTaskRunner extends TaskRunner { } private preventPrototypePollution() { - if (process.env.NODE_ENV === 'test') return; // needed for Jest + // Freeze globals, except for Jest + if (process.env.NODE_ENV !== 'test') { + Object.getOwnPropertyNames(globalThis) + // @ts-expect-error globalThis does not have string in index signature + // eslint-disable-next-line @typescript-eslint/no-unsafe-return, @typescript-eslint/no-unsafe-member-access + .map((name) => globalThis[name]) + .filter((value) => typeof value === 'function') + // eslint-disable-next-line @typescript-eslint/no-unsafe-return, @typescript-eslint/no-unsafe-member-access + .forEach((fn) => Object.freeze(fn.prototype)); + } - Object.getOwnPropertyNames(globalThis) - // @ts-expect-error globalThis does not have string in index signature - // eslint-disable-next-line @typescript-eslint/no-unsafe-return, @typescript-eslint/no-unsafe-member-access - .map((name) => globalThis[name]) - .filter((value) => typeof value === 'function') - // eslint-disable-next-line @typescript-eslint/no-unsafe-return, @typescript-eslint/no-unsafe-member-access - .forEach((fn) => Object.freeze(fn.prototype)); + // Freeze internal classes + [Workflow, Expression, WorkflowDataProxy, DateTime, Interval, Duration] + .map((constructor) => constructor.prototype) + .forEach(Object.freeze); } async executeTask( @@ -478,7 +485,7 @@ export class JsTaskRunner extends TaskRunner { * @param dataProxy The data proxy object that provides access to built-ins * @param additionalProperties Additional properties to add to the context */ - private buildContext( + buildContext( taskId: string, workflow: Workflow, node: INode, diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 3b8af7dd40..5c449a3d92 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -693,6 +693,9 @@ importers: lodash: specifier: 'catalog:' version: 4.17.21 + luxon: + specifier: 'catalog:' + version: 3.4.4 n8n-core: specifier: workspace:* version: link:../../core @@ -709,9 +712,6 @@ importers: '@types/lodash': specifier: 'catalog:' version: 4.14.195 - luxon: - specifier: 'catalog:' - version: 3.4.4 packages/@n8n_io/eslint-config: devDependencies: