fix(core): Prevent prototype pollution in task runner (#12588)

This commit is contained in:
Iván Ovejero 2025-01-15 09:51:42 +01:00 committed by GitHub
parent 674ba3c59a
commit bdf266cf55
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 99 additions and 5 deletions

View file

@ -6,6 +6,7 @@
"command": "/usr/local/bin/node",
"args": [
"--disallow-code-generation-from-strings",
"--disable-proto=delete",
"/usr/local/lib/node_modules/n8n/node_modules/@n8n/task-runner/dist/start.js"
],
"allowed-env": [

View file

@ -1342,4 +1342,75 @@ describe('JsTaskRunner', () => {
task.cleanup();
});
});
describe('prototype pollution prevention', () => {
const checkPrototypeIntact = () => {
const obj: Record<string, unknown> = {};
expect(obj.maliciousKey).toBeUndefined();
};
test('Object.setPrototypeOf should no-op for local object', async () => {
checkPrototypeIntact();
const outcome = await executeForAllItems({
code: `
const obj = {};
Object.setPrototypeOf(obj, { maliciousKey: 'value' });
return [{ json: { prototypeChanged: obj.maliciousKey !== undefined } }];
`,
inputItems: [{ a: 1 }],
});
expect(outcome.result).toEqual([wrapIntoJson({ prototypeChanged: false })]);
checkPrototypeIntact();
});
test('Reflect.setPrototypeOf should no-op for local object', async () => {
checkPrototypeIntact();
const outcome = await executeForAllItems({
code: `
const obj = {};
Reflect.setPrototypeOf(obj, { maliciousKey: 'value' });
return [{ json: { prototypeChanged: obj.maliciousKey !== undefined } }];
`,
inputItems: [{ a: 1 }],
});
expect(outcome.result).toEqual([wrapIntoJson({ prototypeChanged: false })]);
checkPrototypeIntact();
});
test('Object.setPrototypeOf should no-op for incoming object', async () => {
checkPrototypeIntact();
const outcome = await executeForAllItems({
code: `
const obj = $input.first();
Object.setPrototypeOf(obj, { maliciousKey: 'value' });
return [{ json: { prototypeChanged: obj.maliciousKey !== undefined } }];
`,
inputItems: [{ a: 1 }],
});
expect(outcome.result).toEqual([wrapIntoJson({ prototypeChanged: false })]);
checkPrototypeIntact();
});
test('Reflect.setPrototypeOf should no-op for incoming object', async () => {
checkPrototypeIntact();
const outcome = await executeForAllItems({
code: `
const obj = $input.first();
Reflect.setPrototypeOf(obj, { maliciousKey: 'value' });
return [{ json: { prototypeChanged: obj.maliciousKey !== undefined } }];
`,
inputItems: [{ a: 1 }],
});
expect(outcome.result).toEqual([wrapIntoJson({ prototypeChanged: false })]);
checkPrototypeIntact();
});
});
});

View file

@ -103,6 +103,20 @@ export class JsTaskRunner extends TaskRunner {
allowedBuiltInModules: parseModuleAllowList(jsRunnerConfig.allowedBuiltInModules ?? ''),
allowedExternalModules: parseModuleAllowList(jsRunnerConfig.allowedExternalModules ?? ''),
});
this.preventPrototypePollution();
}
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));
}
async executeTask(
@ -203,8 +217,11 @@ export class JsTaskRunner extends TaskRunner {
signal.addEventListener('abort', abortHandler, { once: true });
const preventPrototypeManipulation =
'Object.getPrototypeOf = () => ({}); Reflect.getPrototypeOf = () => ({}); Object.setPrototypeOf = () => false; Reflect.setPrototypeOf = () => false;';
const taskResult = runInContext(
`globalThis.global = globalThis; module.exports = async function VmCodeWrapper() {${settings.code}\n}()`,
`globalThis.global = globalThis; ${preventPrototypeManipulation}; module.exports = async function VmCodeWrapper() {${settings.code}\n}()`,
context,
{ timeout: this.taskTimeout * 1000 },
) as Promise<TaskResultData['result']>;

View file

@ -118,13 +118,14 @@ describe('TaskRunnerProcess', () => {
expect(options.env).not.toHaveProperty('NODE_OPTIONS');
});
it('should use --disallow-code-generation-from-strings flag', async () => {
it('should use --disallow-code-generation-from-strings and --disable-proto=delete flags', async () => {
jest.spyOn(authService, 'createGrantToken').mockResolvedValue('grantToken');
await taskRunnerProcess.start();
expect(spawnMock.mock.calls[0].at(1)).toEqual([
'--disallow-code-generation-from-strings',
'--disable-proto=delete',
expect.stringContaining('/packages/@n8n/task-runner/dist/start.js'),
]);
});

View file

@ -106,9 +106,13 @@ export class TaskRunnerProcess extends TypedEmitter<TaskRunnerProcessEventMap> {
startNode(grantToken: string, taskBrokerUri: string) {
const startScript = require.resolve('@n8n/task-runner/start');
return spawn('node', ['--disallow-code-generation-from-strings', startScript], {
return spawn(
'node',
['--disallow-code-generation-from-strings', '--disable-proto=delete', startScript],
{
env: this.getProcessEnvVars(grantToken, taskBrokerUri),
});
},
);
}
@OnShutdown()