FORBIDDEN: Truncating code or writing placeholders such as "// ... existing code ..." or "# rest unchanged". Every edit is complete and applies as written. FORBIDDEN: Reporting a check as passed without showing the command and its output. REQUIRED: Reason through the rules below before the first edit; when two rules conflict, the one stated first wins.
- Never set
shell: trueto make a command "work". It reintroduces the parser you just removed. If you need a pipeline, build it with two processes and connect their streams rather than handing a string tosh -c. - Never try to sanitise your way to safety by stripping metacharacters. The set differs per shell, quoting rules are subtle, and encodings differ. Escaping helpers such as
shlex.quoteexist for the case where a shell is genuinely unavoidable — treat that as a last resort, not a default. - Never let input decide which binary runs. Allow-list the command:
- Never resolve the binary through
PATHin a privileged context.PATHmay be attacker-influenced. Use an absolute path —/usr/bin/convert. - Reset the child environment rather than inheriting it.LD_PRELOAD,PYTHONPATH,NODE_OPTIONSandIFSall change behaviour:
#Purpose
Rules for invoking external processes safely. The rule underneath everything: never let user input reach a shell.
A shell interprets ;, |, &, `, $(), >, <, && and newline as
control characters. Remove the shell and almost every injection vector goes with
it.
#Pass arguments as an array, never a string
js// WRONG — a shell parses this. `file` can contain `; rm -rf /`
exec(`convert ${file} out.png`);
// RIGHT — no shell; argv[1] is the filename even if it contains `;`
execFile("convert", [file, "out.png"]);
The array form is safe because the operating system receives an argument vector directly. There is no parsing step for input to escape from.
| Language | Dangerous | Safe |
|---|---|---|
| Node | exec, execSync, spawn(…, { shell: true }) | execFile, spawn with an array |
| Python | os.system, subprocess.run(..., shell=True) | subprocess.run([...]) |
| Ruby | system("cmd #{x}"), backticks | system("cmd", x) |
| Go | exec.Command("sh", "-c", s) | exec.Command("cmd", args...) |
| PHP | system, exec, shell_exec | proc_open with an array |
| Java | Runtime.exec(String) | ProcessBuilder(List<String>) |
Never set shell: true to make a command "work". It reintroduces the parser
you just removed. If you need a pipeline, build it with two processes and connect
their streams rather than handing a string to sh -c.
Never try to sanitise your way to safety by stripping metacharacters. The set
differs per shell, quoting rules are subtle, and encodings differ. Escaping
helpers such as shlex.quote exist for the case where a shell is genuinely
unavoidable — treat that as a last resort, not a default.
#Arguments that start with a dash
Even with an argument array, a value beginning with - may be read as an option:
js// A file literally named "--output" changes what the program does
execFile("grep", [pattern, file]);
Two defences, used together:
js// 1. End option parsing explicitly
execFile("grep", ["--", pattern, file]);
// 2. Force a path to be a path
execFile("grep", ["--", pattern, path.resolve(dir, file)]);
Most GNU tools honour --. Where a program does not, prefix relative paths with
./ so they cannot be read as flags.
#Choosing the program itself
- Never let input decide which binary runs. Allow-list the command:
jsconst ALLOWED = { thumbnail: "convert", probe: "ffprobe" };
const bin = ALLOWED[req.body.action];
if (!bin) throw new Error("unsupported action");
- Never resolve the binary through
PATHin a privileged context.PATHmay be attacker-influenced. Use an absolute path —/usr/bin/convert. - Reset the child environment rather than inheriting it.
LD_PRELOAD,PYTHONPATH,NODE_OPTIONSandIFSall change behaviour:
jsexecFile("/usr/bin/convert", ["--", input, output], {
env: { PATH: "/usr/bin:/bin" }, // explicit, minimal
timeout: 10_000,
maxBuffer: 1024 * 1024,
cwd: workDir,
});
#Indirect injection
Command injection frequently arrives through something other than a command string:
- Filenames. A user-supplied name reaching
tar,zip,git, or a shell glob. Generate server-side names; never persist the client's. gitarguments. A branch or remote beginning with--upload-pack=executes a program. Validate against^[A-Za-z0-9._/-]+$and reject leading-.- Archive extraction. Entries may contain
../or absolute paths, or be symlinks pointing outside the destination — seeSecurity/path-traversal. - Environment values interpolated into a script by a later stage.
#Reducing the blast radius
Assume the guard fails and limit what a successful injection achieves:
- Run as an unprivileged user; never
root. - Set
timeoutandmaxBufferon every child process. Unbounded output and never-exiting children are denial of service. - Confine to a container, a
chroot, or a sandbox with no network access when the tool does not need one. - Give the process a working directory containing only what it needs.
- Never return raw
stderrto the user — it leaks paths, versions and arguments. Log it, return something generic.
#Anti-patterns
| Anti-pattern | Why it fails | Fix |
|---|---|---|
exec(\cmd ${input}`)` | A shell parses ;, |, $() | execFile("cmd", [input]) |
spawn(cmd, args, { shell: true }) | Reintroduces the parser | Drop shell: true |
Stripping ; and | from input | Metacharacter sets differ per shell | Remove the shell |
| Input selects the binary | Arbitrary program execution | Allow-list the command |
Relying on PATH when privileged | PATH may be attacker-influenced | Absolute binary path |
| Inheriting the full environment | LD_PRELOAD, IFS, NODE_OPTIONS | Explicit minimal env |
Passing a filename that may start with - | Read as an option | Use -- and path.resolve |
No timeout or maxBuffer | Hung or flooding children | Set both |
Returning stderr to the client | Leaks paths and versions | Log it; return generic |
#Checklist
- No
exec,system,shell_execorshell: truereceives input - Every invocation passes an argument array
-
--terminates options where the program supports it - Paths are resolved before being passed as arguments
- The binary is chosen from a server-side allow-list, by absolute path
- The child environment is set explicitly, not inherited
-
timeout,maxBufferandcwdare configured - The process runs unprivileged and, where possible, sandboxed
- Filenames and
gitrefs are validated and never taken from the client verbatim -
stderris logged server-side and never returned to the caller