Command injection is what happens when user-controlled input reaches a shell instead of staying data. It tends to survive in codebases longer than SQL injection because the trigger is less obvious — there's no query string to eyeball, just a function call that looks like ordinary process orchestration. A reviewer who isn't specifically looking for it can read straight past the line that matters.
The functions that deserve a second look
- Node's
child_process.execandexecSync, which run their argument through a real shell — versusexecFile/spawn, which don't unlessshell: trueis set. - Python's
os.system,subprocess.run(..., shell=True), andos.popen. - PHP's
shell_exec,exec,system,passthru, and the backtick operator. - Ruby's backticks,
system, and%x. - Any call in any language that builds a shell command string with interpolation or concatenation before handing it to an execution function.
The rule isn't "never use these" — it's that the moment any argument includes a value that traces back to a request, a filename, an environment variable set by another process, or a webhook payload, the call needs to not go through a shell at all.
Why the shell is the actual vulnerability
The danger isn't running an external program — it's handing a string to something that interprets metacharacters. A shell treats ;, |, &&, $(), and backticks as control syntax, not literal characters. An attacker who can influence one argument in an otherwise-safe command can use those characters to chain on an entirely separate command. Passing arguments as an array to a non-shell execution function (execFile, subprocess.run without shell=True) sidesteps the entire class, because there is no shell around to interpret anything.
Where it hides in plain code
The obvious case — a search box piped straight into system() — rarely makes it to production anymore. The versions that do ship look mundane: an image-processing pipeline that shells out to convert or ffmpegwith a user-supplied filename, a "test connection" admin feature that pings a hostname the user typed in, a git integration that runs git clone against a repository URL from a webhook, or a report generator that shells out to pandoc or wkhtmltopdf with a path built from request data. None of these look like an injection point on a quick read — they look like normal use of a command-line tool.
Allowlisting beats escaping
Escaping shell metacharacters correctly is genuinely hard to get right across platforms, and a single missed character reopens the hole. Where the set of legal values is small — a file format, a compression level, a predefined operation — validate against an explicit allowlist and reject anything else before it gets near a command. Where a real filesystem path is unavoidable, resolve it and confirm it stays inside an expected directory rather than trusting the string as given.
Checklist for reviewing a diff that shells out
- Prefer an argument-array execution API over a shell-interpreted one; if a shell is genuinely required, every interpolated value is escaped for that specific shell, not string-concatenated.
- Any value in the command that originates from a request, filename, header, or another service is validated against an allowlist before use, not just checked for "reasonable-looking" input.
- Third-party CLI wrappers (image conversion, PDF generation, git operations, archive tools) get the same scrutiny as a raw
execcall — they're shelling out on the library's behalf. - Environment variables passed to a subprocess are an explicit, minimal set rather than an inherited copy of the parent process's environment.