Skip to content

CWE-88: Improper Neutralization of Argument Delimiters in a Command ('Argument Injection') - JavaScript

Overview

Node's child_process.spawn and execFile take the arguments as an array and, with the default shell: false, never involve a shell. That closes command injection. Argument injection is untouched: the array elements are the program's argv, and an element beginning with - is an option.

The distinction matters in Node because the usual advice - "use spawn, not exec" - is about the shell, and teams reasonably read a spawn call with an array as already safe.

Common Vulnerable Patterns

User-controlled clone target directory

const { spawn } = require('node:child_process');

app.post('/clone-repo', (req, res) => {
    const workspace = req.body.name;

    // VULNERABLE - array form stops the shell, not git's own option parsing
    const git = spawn('git', ['clone', 'ssh://git@internal/repo.git', workspace]);

    // Attack: name=--upload-pack=touch /tmp/pwned
    // Result: git runs the named program as the remote "pack" helper

    git.on('close', () => res.send('Cloned'));
});

Why this is vulnerable: git clone accepts --upload-pack=<program> and runs it to serve the remote end. The value arrives as one array element with no metacharacters required, so nothing about quoting or escaping is relevant. Because it is parsed as an option, the destination argument goes missing - git falls back to the repository's basename and proceeds. Verified on git 2.49.0: the named program ran.

Two constraints, both of which have to hold before the payload does anything:

  • --upload-pack only reaches a transport that uses it - ssh://, file://, or a local path with --no-local. Over https:// git answers warning: setting remote service path not supported by protocol and ignores it, so the same injection against an HTTPS clone is a weaker finding.
  • The repository argument has to survive. Injecting into the repository slot instead, as spawn('git', ['clone', repoUrl]), leaves git with no repository at all: fatal: You must specify a repository to clone, exit 129, nothing executed. That is the shape most write-ups quote, and it does not fire from a single element - -c protocol.ext.allow=always ext::sh -c whoami fails the same way, and ext:: is denied by default besides.

execFile treated as inherently safe

const { execFile } = require('node:child_process');

// VULNERABLE - execFile avoids the shell and still passes attacker options
function archive(userFile, res) {
    execFile('tar', ['-cf', 'backup.tar', 'manifest.json', userFile], (err) => {
        res.send(err ? 'failed' : 'ok');
    });
}

// Attack: userFile=--use-compress-program=touch /tmp/pwned
// Result: tar runs the named program in place of a compressor

Why this is vulnerable: execFile differs from exec only in not spawning a shell. The program still parses the array, so a leading - still selects an option - here naming a program for tar to run. Measured on GNU tar 1.35: the program ran and the request returned ok.

The fixed manifest.json argument is what makes it work. With userFile as the only non-option argument, tar exits 2 with "Cowardly refusing to create an empty archive" before the option takes effect - so the same bug in a single-file handler is a denial of service rather than execution, and testing it with one argument is how a real finding gets closed as unreproducible.

Secure Patterns

Validate the parsed value, then pass it after --

const { spawn } = require('node:child_process');

const ALLOWED_HOSTS = new Set(['github.com', 'gitlab.com']);
const REPO_PATH = /^\/[A-Za-z0-9][\w.-]*\/[A-Za-z0-9][\w.-]*(\.git)?$/;

// SECURE - parsed, allowlisted, and passed where options are no longer accepted
function cloneRepo(rawUrl) {
    let url;
    try {
        url = new URL(rawUrl);
    } catch {
        throw new Error('Invalid URL');
    }

    if (url.protocol !== 'https:') throw new Error('Only HTTPS is allowed');
    if (!ALLOWED_HOSTS.has(url.hostname)) throw new Error('Host not allowed');
    if (!REPO_PATH.test(url.pathname)) throw new Error('Invalid repository path');

    return spawn('git', ['clone', '--', url.href], {
        shell: false,
        timeout: 30_000,
        env: { ...process.env, GIT_TERMINAL_PROMPT: '0' },
    });
}

Why this works: Constructing a URL and then checking url.protocol and url.hostname decides on the parsed value rather than on the raw string, so a value that looks acceptable to a regex but parses to a different host is rejected. A parsed https: URL cannot begin with -, which is what removes the option-injection path; -- covers the case where the validation is later loosened. GIT_TERMINAL_PROMPT=0 stops git blocking on a credential prompt for a private repository, and timeout stops a slow clone holding the handler open.

Prefer the platform API where one exists

// SECURE - no argv at all
const response = await fetch(url, { redirect: 'error' });
const body = await response.text();

Why this works: Node has had a global fetch since 18, so shelling out to curl to retrieve a URL has no remaining justification. Passing the URL as a value to an API removes the argument vector, and with it the weakness - redirect: 'error' keeps a host allowlist meaningful by refusing to follow a redirect off the allowed host (CWE-918).

Considerations

  • shell: false answers a different question. It is the fix for CWE-78, and it is already the default for spawn and execFile. A finding on this CWE is not resolved by pointing at it.
  • Where the value lands decides whether this is real. A value in an option's value slot (['--name', value]) is a weaker case than a value in a bare positional slot, and a value that is never an argument is not this weakness at all. Trace the array before judging.
  • shell-quote and similar escaping libraries are the wrong tool. They build shell strings; the safe code here has no shell. Escaping an array element inserts literal characters into the filename and leaves the option parsing exactly as it was.
  • Check what the target binary offers an attacker. git, curl and tar all have options that write files or execute helpers; GTFOBins lists them per binary. A tool whose reachable options are inert is a defensible false positive - record the reason with it.
  • Ask how the tool spells an option's value. One array element is one argv entry, so only an option carrying its own value - --opt=value, or an attached short option such as -K/path - can be delivered by a single injection. ffmpeg has no --opt=value form at all, so -f data arrives as one unrecognised option and every dangerous option there needs a second element the attacker does not control. Measured on 8.0.1, a lone injected element into an ffmpeg argument list is a failed conversion, not a file write.

Testing

The spawn call reads the same before and after the fix, so a re-scan cannot distinguish them. Test the behaviour.

  • Send --upload-pack=touch /tmp/pwned (or your tool's equivalent) and assert both that the request is rejected and that /tmp/pwned does not exist. Check it fires against the unfixed handler first: over an https:// remote git ignores this option, so the assertion would pass either way.
  • Send --help and assert rejection. Most tools exit 0 for it, so this is the payload most likely to be misread as a pass.
  • Send https://github.com.attacker.example/a/b and assert rejection - a substring check on the hostname accepts it, URL parsing plus a Set lookup does not.
  • Assert legitimate values still work: repository names containing hyphens and dots, URLs with a .git suffix and without. This is where tightened validation breaks users, and no scanner will report it.

Additional Resources