Skip to content

Commit

Permalink
fix: throw an error in case of unknown properties or inherited proper…
Browse files Browse the repository at this point in the history
…ties in `workerOpts`, `workerThreadOpts` and `forkOpts` to protect against security issues related to prototype pollution
  • Loading branch information
josdejong committed Oct 25, 2023
1 parent 99ea396 commit ea5368c
Show file tree
Hide file tree
Showing 3 changed files with 117 additions and 0 deletions.
10 changes: 10 additions & 0 deletions src/WorkerHandler.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

var Promise = require('./Promise');
var environment = require('./environment');
const {validateOptions, forkOptsNames, workerThreadOptsNames, workerOptsNames} = require("./validateOptions");

/**
* Special message sent by parent which causes a child process worker to terminate itself.
Expand Down Expand Up @@ -86,6 +87,9 @@ function setupWorker(script, options) {
}

function setupBrowserWorker(script, workerOpts, Worker) {
// validate the options right before creating the worker (not when creating the pool)
validateOptions(workerOpts, workerOptsNames, 'workerOpts')

// create the web worker
var worker = new Worker(script, workerOpts);

Expand All @@ -103,6 +107,9 @@ function setupBrowserWorker(script, workerOpts, Worker) {
}

function setupWorkerThreadWorker(script, WorkerThreads, workerThreadOptions) {
// validate the options right before creating the worker thread (not when creating the pool)
validateOptions(workerThreadOptions, workerThreadOptsNames, 'workerThreadOpts')

var worker = new WorkerThreads.Worker(script, {
stdout: false, // automatically pipe worker.STDOUT to process.STDOUT
stderr: false, // automatically pipe worker.STDERR to process.STDERR
Expand All @@ -126,6 +133,9 @@ function setupWorkerThreadWorker(script, WorkerThreads, workerThreadOptions) {
}

function setupProcessWorker(script, options, child_process) {
// validate the options right before creating the child process (not when creating the pool)
validateOptions(options.forkOpts, forkOptsNames, 'forkOpts')

// no WorkerThreads, fallback to sub-process based workers
var worker = child_process.fork(
script,
Expand Down
51 changes: 51 additions & 0 deletions src/validateOptions.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
/**
* Validate that the object only contains known option names
* - Throws an error when unknown options are detected
* - Throws an error when some of the allowed options are attached
* @param {Object | undefined} options
* @param {string[]} allowedOptionNames
* @param {string} objectName
* @retrun {Object} Returns the original options
*/
exports.validateOptions = function validateOptions(options, allowedOptionNames, objectName) {
if (!options) {
return
}

var optionNames = options ? Object.keys(options) : []

// check for unknown properties
var unknownOptionName = optionNames.find(optionName => !allowedOptionNames.includes(optionName))
if (unknownOptionName) {
throw new Error('Object "' + objectName + '" contains an unknown option "' + unknownOptionName + '"')
}

// check for inherited properties which are not present on the object itself
var illegalOptionName = allowedOptionNames.find(allowedOptionName => {
return Object.prototype[allowedOptionName] && !optionNames.includes(allowedOptionName)
})
if (illegalOptionName) {
throw new Error('Object "' + objectName + '" contains an inherited option "' + illegalOptionName + '" which is ' +
'not defined in the object itself but in its prototype. Only plain objects are allowed. ' +
'Please remove the option from the prototype or override it with a value "undefined".')
}

return options
}

// source: https://developer.mozilla.org/en-US/docs/Web/API/Worker/Worker
exports.workerOptsNames = [
'credentials', 'name', 'type' ]

// source: https://nodejs.org/api/child_process.html#child_processforkmodulepath-args-options
exports.forkOptsNames = [
'cwd', 'detached', 'env', 'execPath', 'execArgv', 'gid', 'serialization',
'signal', 'killSignal', 'silent', 'stdio', 'uid', 'windowsVerbatimArguments',
'timeout'
]

// source: https://nodejs.org/api/worker_threads.html#new-workerfilename-options
exports.workerThreadOptsNames = [
'argv', 'env', 'eval', 'execArgv', 'stdin', 'stdout', 'stderr', 'workerData',
'trackUnmanagedFds', 'transferList', 'resourceLimits', 'name'
]
56 changes: 56 additions & 0 deletions test/Pool.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1214,4 +1214,60 @@ describe('Pool', function () {
assert(handlerCalled === false);
});
});

describe('validate', () => {
it('should not allow unknown properties in forkOpts', function() {
var pool = createPool({
workerType: 'process',
forkOpts: { foo: 42 }
});

assert.throws(function () {
pool.exec(add, [3, 4]);
}, /Error: Object "forkOpts" contains an unknown option "foo"/);
});

it('should not allow inherited properties in forkOpts', function() {
var pool = createPool({
workerType: 'process'
});

// prototype pollution
Object.prototype.env = { NODE_OPTIONS: '--inspect-brk=0.0.0.0:1337' };

assert.throws(function () {
pool.exec(add, [3, 4]);
}, /Error: Object "forkOpts" contains an inherited option "env"/);

delete Object.prototype.env;
after(() => { delete Object.prototype.env });
});

it('should not allow unknown properties in workerThreadOpts', function() {
var pool = createPool({
workerType: 'thread',
workerThreadOpts: { foo: 42 }
});

assert.throws(function () {
pool.exec(add, [3, 4]);
}, /Error: Object "workerThreadOpts" contains an unknown option "foo"/);
});

it('should not allow inherited properties in workerThreadOpts', function() {
var pool = createPool({
workerType: 'thread'
});

// prototype pollution
Object.prototype.env = { NODE_OPTIONS: '--inspect-brk=0.0.0.0:1337' };

assert.throws(function () {
pool.exec(add, [3, 4]);
}, /Error: Object "workerThreadOpts" contains an inherited option "env"/);

delete Object.prototype.env;
after(() => { delete Object.prototype.env });
});
});
});

0 comments on commit ea5368c

Please sign in to comment.