Fix OS command and argument injection in Node.js and Express

1. Find every child_process.exec, execSync and spawn/execFile called with { shell: true } whose command is built from req.query, req.params or req.body.
2. Replace it with execFile/execFileSync or spawn (no shell), passing the program and an array of arguments: execFileSync(tool, ['--', name]).
3. Put a literal '--' in the array before the first user-controlled operand, so a value beginning with '-' is treated as an operand and not as an option.
4. For any argument that selects the command, a subcommand, a flag or an enumerated option value, map the caller's choice through an allowlist (a Map) and answer 400 for anything else.
5. Check each query-string value is a single string before using it; Express 5's default 'simple' parser returns an array when a key repeats.
6. Add regression tests with this card's payloads (separators, substitution, newline, and a leading-dash option) that assert a marker file was never created, next to functional tests for names with spaces, apostrophes and quotes.
7. Log the tool's stderr on the server and return a generic 500; its error text helps an attacker tune a payload.

The three implementations share one API and one test suite; only the code below changes. The tests load the one that IMPL names.

export function greet(tool, name) {
  // execSync runs the string through /bin/sh -c: every shell metacharacter in
  // `name` is live, and `; | && $( ) backticks` and newlines all start new work.
  return execSync(`${tool} ${name}`, { encoding: 'utf8' });
}

export function greet(tool, name) {
  // execFileSync runs the program directly - no shell parses the arguments - and
  // "--" means everything after it is an operand, so a name that starts with a
  // dash cannot pose as an option. Metacharacters are just characters here.
  return execFileSync(tool, ['--', name], { encoding: 'utf8' });
}

// The format picks part of the command itself, which a value can never do
// safely, so map the caller's choice onto a fixed flag value from an allowlist.
const FORMAT_FLAGS = new Map([
  ['text', 'text'],
  ['json', 'json'],
]);

export function report(tool, name, format) {
  const flag = FORMAT_FLAGS.get(format);
  if (flag === undefined) {
    throw new InputRejected('unsupported format');
  }
  return execFileSync(tool, ['--format', flag, '--', name], { encoding: 'utf8' });
}

// The "obvious" shell separators. Note what is missing: $ ( ) and backticks
// (command substitution), and the leading dash of an injected option.
const BLOCKED = /[;|&<>\n]/;

function shellQuote(value) {
  if (BLOCKED.test(value)) throw new InputRejected('illegal character in input');
  return `"${value}"`;
}

export function greet(tool, name) {
  return execSync(`${tool} ${shellQuote(name)}`, { encoding: 'utf8' });
}

The exploit tests send the attack and check that it fails; functional tests next to them keep ordinary requests working, so a fix that rejects everything fails too.

describe(`exploit (IMPL=${implName})`, () => {
  // OS command injection: a shell metacharacter in the name must stay data.
  for (const [label, payload] of [
    ['a semicolon does not start a second command', 'Alice; touch MARKER'],
    ['a pipe does not start a second command', 'Alice | touch MARKER'],
    ['an && chain does not start a second command', 'Alice && touch MARKER'],
    ['a $( ) substitution does not run', 'Alice$(touch MARKER)'],
    ['backticks do not run a substitution', 'Alice`touch MARKER`'],
    ['a newline does not start a second command', 'Alice\ntouch MARKER'],
  ]) {
    test(label, async () => {
      const { dir, marker } = freshMarker();
      await assertNoMarker('/greeting', { name: payload.replace('MARKER', marker) }, marker, dir);
    });
  }

  // Argument injection: a name that starts with a dash must not pose as an option.
  test('a leading --output= cannot make the tool write a file', async () => {
    const { dir, marker } = freshMarker();
    await assertNoMarker('/greeting', { name: `--output=${marker}` }, marker, dir);
  });

  test('a leading -o cannot make the tool write a file', async () => {
    const { dir, marker } = freshMarker();
    await assertNoMarker('/greeting', { name: `-o${marker}` }, marker, dir);
  });

  // The allowlisted format cannot smuggle in an extra option...
  test('an option injected through the format is rejected', async () => {
    const { dir, marker } = freshMarker();
    await assertNoMarker('/report', { name: 'Alice Martin', format: `text --output=${marker}` }, marker, dir);
  });

  // ...and the operand after "--" cannot either, even with a valid format.
  test('a leading-dash name is an operand, not an option', async () => {
    const { dir, marker } = freshMarker();
    await assertNoMarker('/report', { name: `--output=${marker}`, format: 'text' }, marker, dir);
  });
});

• spawn/execFile with { shell: true } re-introduces the shell and re-opens the hole; the safe default is shell: false (the default).
• execFile still runs exactly one program, but without '--' an operand that starts with '-' is parsed as an option. Argument injection (CWE-88) needs no shell at all.
• A template literal is string building, not escaping. child_process.exec has no parameter binding; the array form of execFile/spawn is the binding.
• Passing the whole command as a single spawn argument with shell: true is the same bug wearing an array.
• node:child_process has no built-in shell-quoting helper, and hand-quoting is exactly the mistake in impl/wrong.js; reach for execFile instead of a quoting library.
• A number or boolean from req.query coerced into the args array becomes '[object Object]' or throws; validate and convert to string deliberately.
• Allowlist by value, not by regex on the raw input: a blocklist of shell metacharacters still lets --output= and -o through, because a dash is not a metacharacter.

1. Run the tests in all three modes: IMPL=fixed must pass; IMPL=vulnerable and IMPL=wrong must fail (node ../../run-all.mjs node-express does this for you).
2. Against your own endpoint, send name=Alice;touch /tmp/pwned and name=--output=/tmp/pwned and confirm /tmp/pwned is never created.
3. Do not rely on Semgrep's detect-child-process rule here: it matches the unprefixed 'child_process' module and found nothing on this card's 'node:child_process' import, so the regression tests are the control.

What each rule reported when it was run on this card's code, or what its source shows it covers where it was not run. A rule that exists for the weakness does not always catch it.

Download the project, install its dependencies once (this step needs the network), then run the tests against each implementation from the command-injection/node-expressfolder.

npm ci

# Expected: every test passes
IMPL=fixed npm test --silent

# Expected: an exploit test fails
IMPL=vulnerable npm test --silent

# Expected: a test fails
IMPL=wrong npm test --silent

Each run writes a JUnit report to reports/junit.xml. From the top folder of the download, node run-all.mjs command-injection/node-express runs all three modes and checks the expected results.

Download the project ZIP, 25 KB

Verified

Machine

macOS 27.0 (Darwin 27.0.0, arm64)

Toolchain

• node 26.11.1
• express 5.3.0
• shell /bin/sh (dash-compatible POSIX shell)

Matrix

fixed: 16 of 16 tests pass; vulnerable: 16 of 16 tests fail (10 exploit tests); wrong: 7 of 16 tests fail (5 exploit tests)

Dependency freshness

express 5.3.0 was published 2026-10-09 (one day before verification); taken because it is a security release (qs, body-parser and proxy-addr updates) and the prior 5.2.1 carries those advisories.

Versions and results hold for the toolchain above. The card's tests are the check: run them with your own versions before relying on the fix.

References

1. OWASP OS Command Injection Defense Cheat Sheet
2. Node.js v26 docs: child_process.execFile and the shell option
3. Node.js v26 docs: child_process.spawn (arguments as a list)
4. Express: Migrating to Express 5 (req.query and the default query parser)
5. CWE-78: OS Command Injection
6. CWE-88: Argument Injection
7. CAPEC-88: OS Command Injection
8. CAPEC-6: Argument Injection

Attack patterns:

• CAPEC-88 at MITRE
• CAPEC-6 at MITRE