Skip to content

Commit

Permalink
fix(core): Prevent prototype pollution of internal classes in task ru…
Browse files Browse the repository at this point in the history
…nner (#12610)
  • Loading branch information
tomi authored Jan 15, 2025
1 parent 4a1a999 commit eceee7f
Show file tree
Hide file tree
Showing 4 changed files with 47 additions and 17 deletions.
4 changes: 2 additions & 2 deletions packages/@n8n/task-runner/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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:"
}
}
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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();
});
});
});
29 changes: 18 additions & 11 deletions packages/@n8n/task-runner/src/js-task-runner/js-task-runner.ts
Original file line number Diff line number Diff line change
@@ -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,
Expand Down Expand Up @@ -108,15 +109,21 @@ export class JsTaskRunner extends TaskRunner {
}

private preventPrototypePollution() {
if (process.env.NODE_ENV === 'test') return; // needed for Jest

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 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));
}

// Freeze internal classes
[Workflow, Expression, WorkflowDataProxy, DateTime, Interval, Duration]
.map((constructor) => constructor.prototype)
.forEach(Object.freeze);
}

async executeTask(
Expand Down Expand Up @@ -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,
Expand Down
6 changes: 3 additions & 3 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit eceee7f

Please sign in to comment.