Merge commit from fork

* security: harden Piscina options against prototype pollution

Harden Piscina against prototype pollution by ensuring all
user-controlled option reads come from own properties and by storing
pool options on a null-prototype object.

This prevents attackers who can pollute Object.prototype from
influencing worker configuration (filename, name, transferList,
signal, force) or pool defaults.

NB: pool.options is now a null-prototype object. This is a potential
breaking change for consumers that call Object.prototype methods
directly on pool.options (e.g. pool.options.hasOwnProperty(...)).

* fix: use sanitized options for resourceLimits validation

* fix: sanitize run/close options with withNullPrototype

Use the existing withNullPrototype helper instead of getOwn; address feedback from @metcoder95

* fix: avoid re-linking Object.prototype in Piscina constructor

Author: Fcmam5

src/common.ts

@@ -59,3 +59,9 @@ export function maybeFileURLToPath (filename : string) : string {
 export function getAvailableParallelism () : number {
   return availableParallelism();
 }
+
+// Copy own properties onto a prototype-less object, so that options are never
+// resolved through a polluted prototype chain.
+export function withNullPrototype<T extends object>(source: T, overrides?: Partial<T>): T {
+  return Object.assign(Object.create(null), source, overrides)
+}

src/index.ts

@@ -48,6 +48,7 @@ import {
   markMovable,
   getAvailableParallelism,
   maybeFileURLToPath,
+  withNullPrototype,
 } from './common';
 const cpuParallelism : number = getAvailableParallelism();
 
@@ -185,7 +186,12 @@ class ThreadPool {
 
     const filename =
       options.filename ? maybeFileURLToPath(options.filename) : null;
-    this.options = { ...kDefaultOptions, ...options, filename, maxQueue: 0 };
+    this.options = withNullPrototype({
+      ...kDefaultOptions,
+      ...options,
+      filename,
+      maxQueue: 0,
+    });
 
     if (this.options.recordTiming) {
       this.histogram = new PiscinaHistogramHandler();
@@ -741,8 +747,8 @@ export default class Piscina<Exports extends Record<string, (payload: any) => an
   #histogram: PiscinaHistogram | null = null;
 
   constructor (options : Options = {}) {
-    const opts = { ...options, '__proto__': null };
-    super({ ...opts, name: 'Piscina' });
+    const opts = withNullPrototype(options);
+    super(withNullPrototype(opts, { name: 'Piscina' }));
 
     if (typeof opts.filename !== 'string' && opts.filename != null) {
       throw Errors.ValidationError('options.filename must be a string or null');
@@ -781,7 +787,7 @@ export default class Piscina<Exports extends Record<string, (payload: any) => an
         !['sync', 'async', 'disabled'].includes(opts.atomics))) {
       throw Errors.ValidationError('options.atomics should be a value of sync, sync or disabled.');
     }
-    if (options.resourceLimits != null && typeof options.resourceLimits !== 'object') {
+    if (opts.resourceLimits != null && typeof opts.resourceLimits !== 'object') {
       throw Errors.ValidationError('options.resourceLimits must be an object');
     }
     if (opts.taskQueue != null && !isTaskQueue(opts.taskQueue)) {
@@ -817,12 +823,12 @@ export default class Piscina<Exports extends Record<string, (payload: any) => an
         Errors.ValidationError('options must be an object'));
     }
 
-    const {
-      transferList,
-      signal
-    } = options;
-    const filename = Object.prototype.hasOwnProperty.call(options, 'filename') ? options.filename : null;
-    const name = Object.prototype.hasOwnProperty.call(options, 'name') ? options.name : null;
+    options = withNullPrototype(options);
+
+    const transferList = options.transferList;
+    const signal = options.signal ?? null;
+    const filename = options.filename ?? null;
+    const name = options.name ?? null;
 
     if (transferList !== undefined && !Array.isArray(transferList)) {
       return Promise.reject(
@@ -848,6 +854,8 @@ export default class Piscina<Exports extends Record<string, (payload: any) => an
       throw Errors.ValidationError('options must be an object');
     }
 
+    options = withNullPrototype(options);
+
     let { force } = options;
 
     if (force != null && typeof force !== 'boolean') {

test/option-validation.test.ts

@@ -149,4 +149,60 @@ test('trackUnmanagedFds must be a boolean', () => {
   assert.throws(() => new Piscina(({
     trackUnmanagedFds: 'string'
   }) as any), /options.trackUnmanagedFds must be a boolean/);
-});
+});
+
+test('execArgv is not tampered', async () => {
+  (Object.prototype as any).execArgv = ['--not-a-real-flag']
+
+  const pool = new Piscina({
+    filename: resolve(__dirname, 'fixtures/eval.js'),
+    minThreads: 1,
+    maxThreads: 1
+  })
+
+  try {
+    assert.strictEqual(pool.options.execArgv, undefined)
+    assert.strictEqual(await pool.run('42'), 42)
+  } finally {
+    delete (Object.prototype as any).execArgv
+    await pool.close()
+  }
+})
+
+test('loadBalancer is not tampered', async () => {
+  let called = false
+  ;(Object.prototype as any).loadBalancer = () => { called = true; return null }
+
+  const pool = new Piscina({
+    filename: resolve(__dirname, 'fixtures/eval.js'),
+    minThreads: 1,
+    maxThreads: 1
+  })
+
+  try {
+    assert.strictEqual(pool.options.loadBalancer, undefined)
+    assert.strictEqual(await pool.run('42'), 42)
+    assert.strictEqual(called, false)
+  } finally {
+    delete (Object.prototype as any).loadBalancer
+    await pool.close()
+  }
+})
+
+test('env is not tampered', async () => {
+  (Object.prototype as any).env = { NODE_OPTIONS: '--title=polluted' }
+
+  const pool = new Piscina({
+    filename: resolve(__dirname, 'fixtures/eval.js'),
+    minThreads: 1,
+    maxThreads: 1
+  })
+
+  try {
+    assert.strictEqual(pool.options.env, undefined)
+    assert.strictEqual(await pool.run('42'), 42)
+  } finally {
+    delete (Object.prototype as any).env
+    await pool.close()
+  }
+})