How javascript.lang.security.detect-child-process.detect-child-process happens in Node.js and how to fix it

Introduction

In the packages/runner/src/main.js file of a Node.js runner application, we discovered a high-severity command injection vulnerability at line 82. The run() function accepts a destructured object containing argv—an array of command-line arguments—and passes it directly to child_process.spawn() without any validation.

The vulnerable code pattern looked like this:

let nwProcess = child_process.spawn(nwExe, [...[srcDir], ...argv], {
  stdio: "inherit",
});

This matters because the argv parameter flows from function arguments into a system command execution context. If any upstream code passes user-controllable data into this function, an attacker could inject malicious arguments or, depending on the environment, exploit shell metacharacters to execute arbitrary commands.

The Vulnerability Explained

The run() function in main.js is designed to spawn an NW.js (Node-WebKit) process with specific arguments. It receives a configuration object with properties including version, flavor, platform, arch, srcDir, cacheDir, and critically, argv.

Here's the vulnerable code before the fix:

async function run({
  version,
  flavor,
  platform,
  arch,
  srcDir,
  cacheDir,
  argv,
}) {
  // ... path validation for nwExe ...

  let nwProcess = child_process.spawn(nwExe, [...[srcDir], ...argv], {
    stdio: "inherit",
  });

Why This Is Dangerous

1. No Type Validation: The code assumes argv is an array of strings, but never verifies this. If argv contained objects, functions, or specially crafted values, unexpected behavior could occur.

2. No Shell Flag Specified: While child_process.spawn() defaults to shell: false, explicitly omitting this setting leaves the security posture ambiguous and makes the code vulnerable to future changes or misconfigurations.

3. Direct Parameter Passthrough: The spread operator (...argv) passes all elements directly to the spawned process without sanitization.

Attack Scenario

Consider if an upstream API endpoint allowed users to specify runner options:

// Hypothetical vulnerable upstream code
app.post('/run', (req, res) => {
  run({
    version: req.body.version,
    argv: req.body.argv,  // User-controlled!
    // ...
  });
});

An attacker could send:

{
  "argv": ["--malicious-flag", "; rm -rf /", "--another-arg"]
}

While spawn() without shell mode wouldn't interpret the semicolon, if the code were ever changed to use shell: true, or if argv contained non-string values that coerced unexpectedly, command injection would be possible.

The Fix

The fix implements two critical security controls in packages/runner/src/main.js:

Change 1: Input Validation (Line 79-81)

if (!Array.isArray(argv) || !argv.every((arg) => typeof arg === "string")) {
  throw new Error("Invalid argv: expected an array of strings");
}

This validation ensures:
- argv is definitely an array (not null, undefined, or an object)
- Every element in argv is a string (not objects, numbers, or functions that could cause unexpected behavior)

Change 2: Explicit Shell Disable (Line 87)

let nwProcess = child_process.spawn(nwExe, [...[srcDir], ...argv], {
  stdio: "inherit",
  shell: false,  // Explicitly added
});

Setting shell: false explicitly:
- Documents the security intent
- Prevents accidental shell interpretation if defaults ever change
- Ensures special characters like ;, |, && are treated as literal arguments

Before and After Comparison

Before:

let nwProcess = child_process.spawn(nwExe, [...[srcDir], ...argv], {
  stdio: "inherit",
});

After:

if (!Array.isArray(argv) || !argv.every((arg) => typeof arg === "string")) {
  throw new Error("Invalid argv: expected an array of strings");
}

let nwProcess = child_process.spawn(nwExe, [...[srcDir], ...argv], {
  stdio: "inherit",
  shell: false,
});

Prevention & Best Practices

1. Always Validate Input Types

Before passing any data to child_process functions, validate:
- The data type matches expectations
- The content conforms to expected patterns
- No unexpected characters or values are present

function validateArgs(args) {
  if (!Array.isArray(args)) {
    throw new TypeError('Arguments must be an array');
  }
  return args.every(arg => 
    typeof arg === 'string' && 
    /^[\w\-\.\/]+$/.test(arg)  // Allowlist safe characters
  );
}

2. Never Use shell: true with User Input

When shell: true is set, special characters are interpreted by the shell:

// DANGEROUS - Never do this with user input
child_process.spawn(cmd, args, { shell: true });

// SAFER - Shell interpretation disabled
child_process.spawn(cmd, args, { shell: false });

3. Use execFile Instead of exec

child_process.execFile() doesn't spawn a shell by default, making it safer than exec():

// Preferred for running specific executables
child_process.execFile('/path/to/binary', ['arg1', 'arg2']);

4. Implement Allowlists for Arguments

If possible, validate arguments against a known-good list:

const ALLOWED_FLAGS = ['--verbose', '--debug', '--quiet'];
const safeArgs = argv.filter(arg => ALLOWED_FLAGS.includes(arg));

Key Takeaways

• Always validate argv arrays before passing to child_process.spawn() in Node.js runner applications
• The run() function in main.js now rejects non-string array elements, preventing type confusion attacks
• Explicit shell: false documents security intent and prevents future regressions
• Defense in depth combines input validation with secure spawn options for robust protection
• Private applications still need hardening—this vulnerability could be chained with other weaknesses by automated exploit tools

How Orbis AppSec Detected This

• Source: Function parameter argv passed to the run() function in packages/runner/src/main.js
• Sink: child_process.spawn(nwExe, [...[srcDir], ...argv], {...}) at line 82
• Missing control: No validation that argv is an array of strings; no explicit shell: false setting
• CWE: CWE-78 (Improper Neutralization of Special Elements used in an OS Command)
• Fix: Added type validation for argv array and explicitly set shell: false to prevent command injection

Orbis AppSec automatically detected this vulnerability and opened a pull request with the fix. Try Orbis AppSec on your repositories to find and fix issues like this automatically.

Conclusion

This command injection vulnerability in packages/runner/src/main.js demonstrates why input validation is critical even in private applications. The unvalidated argv parameter flowing directly into child_process.spawn() created an exploit primitive that could be chained with other vulnerabilities.

The two-line fix—validating argv as a string array and explicitly setting shell: false—eliminates this attack surface while maintaining the function's intended behavior. When working with child_process in Node.js, always validate input types, use explicit security settings, and consider allowlisting valid arguments.

References

• CWE-78: Improper Neutralization of Special Elements used in an OS Command
• OWASP Command Injection Prevention Cheat Sheet
• Node.js child_process Documentation
• Semgrep detect-child-process Rule
• harden: sanitize child_process call in main.js