Skip to content

child_process: fix incomplete prototype pollution hardening

Rodrigo Muino Tomonari requested to merge github/fork/lirantal/fix/prototype-hardening-missing-for-spawn into main

Prior pull request (#48726) hardened against prototype pollution vulnerabilities but effectively missed some use-cases which opened a window for prototype pollution for some child_process functions such as spawn(), spawnSync(), and execFileSync().

This PR adds a (failing) test case that confirms the issue and follows-up with a fix for the bug in child_process functions to ensure consistent behavior.

Note: this is not considered a security vulnerability because the Node.js threat model does not recognize prototype pollution as a viable security vulnerability in the runtime.

Expectation vs Actual

Expectation:

  • Per the spawn() API documentation, spawn() should default to shell: false. Similarly, execFile() follows the same.
  • Per the referenced prototype pollution hardedning Pull Request from 2023, the following simulated attack shouldn't work: Object.prototype.shell = true; child_process.spawn('ls', ['-l && touch /tmp/new'])

Actual:

  • Object.prototype.shell = true; child_process.execFile('ls', ['-l && touch /tmp/new']) - ✅ No side effects, hardening works well for.
  • Object.prototype.shell = true; child_process.spawn('ls', ['-l && touch /tmp/new']) - ✅ No side effects, hardening works well for.
  • Object.prototype.shell = true; child_process.execFile('ls', ['-l && touch /tmp/new'], { stdio: 'inherit'}) - ✅ No side effects, hardening works well for.
  • Object.prototype.shell = true; child_process.spawn('ls', ['-l && touch /tmp/new'], { stdio: 'inherit'}) - ❌ Vulnerability manifests, hardening fails.

Note: other APIs are also prone to this issue: execFileSync and spawnSync.

Demo of the inconsistent prototype hardening findings

The following is provided as an easy reproduction and proof-of-concept (POC) for described findings on the prototype mishandling issue:

const { execFile, spawn, spawnSync, execFileSync } = require("child_process");

// Simulate a successful prototype attack impact:
const a = {};
a.__proto__.shell = true;

// Show the prototype is indeed affected:
console.log("Object.shell value:", Object.shell, "\n");

// Node.js user-land code use of various child_process functions:
execFile("ls", ["-l && touch /tmp/from-ExecFile"], {
  stdio: "inherit",
});

spawn("ls", ["-la && touch /tmp/from-Spawn"], {
  stdio: "inherit",
});

execFileSync("ls", ["-l && touch /tmp/from-ExecFileSync"], {
  stdio: "inherit",
});

spawnSync("ls", ["-la && touch /tmp/from-SpawnSync"], {
  stdio: "inherit",
});

The output would be as follows:

$ node app.js
Object.shell value: true

[...]

$ ls -alh /tmp/from*
Permissions Size User     Date Modified Name
.rw-r--r--     0 lirantal  4 Jul 14:14   /tmp/from-ExecFileSync
.rw-r--r--     0 lirantal  4 Jul 14:14   /tmp/from-Spawn
.rw-r--r--     0 lirantal  4 Jul 14:14   /tmp/from-SpawnSync

Demonstrating that while the child_process.exec function is hardened from prototype chain look up of the shell property, this isn't the case for other process execution functions, mainly: spawn, execFileSync, spawnSync.

Merge request reports