Finding every shell call built from user input

The reliable fix for OS command injection is to stop handing user-influenced strings to a shell at all. Better escaping is the weaker option. The practical work is finding every place your code starts a process, then deciding for each one whether it needs to exist.

Command injection happens when an application passes unsafe user data to a system shell, and the injected commands usually run with the application's privileges. CWE-78 adds an uncomfortable detail: because the application executes the commands, the activity may appear to come from the application or its owner.

Why escaping is the wrong first answer

PHP's escapeshellarg() wraps a value in single quotes so the shell treats it as one argument. That stops & echo "hello" from running as a second command. It does not stop the attacker choosing which single argument the program receives.

Every OS command injection is also an argument injection. In the OWASP cheat sheet, escape("--help") passed to curl still prints the help text. Depending on the program, the impact ranges from information disclosure to remote code execution.

A reader note on the PHP manual page for escapeshellarg() shows the same flaw with tar. Each file name is escaped correctly, yet a value starting with a hyphen is still read as an option, and tar has an option that runs another command. Adding -- before the file names fixes it, but the note points out that not every program supports that separator.

On Windows the manual says escapeshellarg() uses double quotes and replaces percent signs, exclamation marks and double quotes with spaces. A reader note adds that the result is safe but lossy. Either way, it is a reason to avoid the shell, not a fix.

Find the calls

Run these over every repository. Adjust the file types to your layout:

rg -n 'shell=True|os\.system|os\.popen' --type py
rg -n "subprocess\.(run|Popen|call|check_output)\(\s*[f\"']" --type py
rg -n '\b(system|exec|shell_exec|passthru|popen|proc_open)\s*\(' --type php
rg -n '`[^`]*\$' --type php
rg -n 'Runtime\.getRuntime\(\)\.exec|ProcessBuilder' --type java
rg -n 'ProcessStartInfo|Process\.Start|UseShellExecute' --type cs
rg -n '\bsystem\s*\(|popen\s*\(|exec[lv]p?e?\s*\(' --type c

The second Python pattern catches a single string passed as the command. Without shell=True that only works when the string names a program and nothing else, so every hit needs a manual look. The PHP backtick pattern is noisy because it also matches backticks in comments and SQL. Also search shell scripts, deploy scripts and cron wrappers, which build commands from variables just as often as application code.

Include hardcoded commands. The OWASP community write-up describes a privileged CGI program that runs cd /var/yp && make with no user input in the string. It used no absolute path for make, so an attacker could change $PATH and have their own binary run with the program's privileges.

Triage the hits

A raw list of hits is not a finding list. For each one, trace the value backwards and ask whether a request can reach it. Route and query parameters, form fields, cookies and HTTP headers are the obvious sources. Uploaded file names, values from a database that users can write to, queue messages and environment variables are the ones people forget.

Sort the hits into three piles: reachable from an unauthenticated request, reachable only after login, and not reachable at all. Fix the first pile first. Keep the third on the list, because a later refactor can connect it to a request without anyone noticing.

Replace the call with an API

Built-in library functions cannot be manipulated into doing anything other than their intended task. Use mkdir() instead of system("mkdir /dir_name"). Use the Java mail API in javax.mail.* instead of shelling out to mail. CWE-78 gives the same advice for design: use library calls rather than external processes.

When you must start a process

Some tools have no library equivalent. Then use two layers together: an argument list with no shell, and validation.

The Python subprocess documentation says a sequence of arguments is generally preferred, because the module handles escaping and quoting. It carries a note to read the Security Considerations section before using shell=True. It also recommends a fully qualified path for the executable, which removes the PATH lookup that the make example above abuses.

import re, subprocess

if not re.fullmatch(r"[a-z0-9]{3,10}", name):
    raise ValueError("rejected")

subprocess.run(["/usr/bin/tar", "cf", "out.tar", "--", name], check=True)

The pattern already rejects a leading hyphen, so -- looks redundant. It is defence in depth: if someone later loosens the regex, the separator still stops name being read as an option.

In .NET, use a fixed, fully qualified path in FileName, set UseShellExecute = false and add each argument through ArgumentList. Never pass untrusted input to cmd.exe, powershell.exe or sh -c. Separating arguments is not validation, in Java or .NET.

Then validate the values. Check the command against a list of allowed commands, and check each argument against allowed values or a pattern with a maximum length, with metacharacters and whitespace kept out.

For curl, -- is not enough, because later arguments are still URL operands and further transfers are possible. Pass exactly one validated URL, use --globoff, and apply SSRF protections.

Run the application with the lowest privileges it needs, and give single-task jobs their own isolated account. A sandbox such as chroot, AppArmor or SELinux helps, but CWE-78 rates its effectiveness as limited. It narrows what an attacker can reach and does not remove the flaw.

Where to start

1. Run the searches above over every repository, including deploy scripts, CI jobs and cron wrappers. List each hit with the input that reaches it, and sort the list by whether a request can reach it.
2. For each hit, ask whether a library or built-in function does the same job. If it does, replace the call and delete the process start.
3. Rewrite the remaining calls to use an argument list, no shell and an absolute executable path. Add allowlist validation and the -- separator where the program supports it.
4. Run the processes under a dedicated low-privilege account, so a missed case has less to work with.
5. Test each remaining call with a value like bob.txt;sleep 5 and with a value that begins with a hyphen. Time the response rather than waiting for output. The hyphen-prefixed value should be rejected or treated as a file name. For an outside check, a penetration test will try both, or our secure software development team can review the code with you.