Why is ProcessBuilder with an argument array safer than Runtime.exec with a shell string for running external programs?

## The vulnerability: OS command injection **OS command injection** is when an attacker gets your program to run operating-system commands of their choosing, by injecting text into a command your program executes. It is the shell-equivalent of SQL injection, and it often gives full control of the host. **The role of the shell.** A *shell* (bash, sh, cmd) is a program that parses a command *line* and interprets special characters called **metacharacters**: - `;` separates commands; - `|` pipes one command's output into another; - `&&` / `||` run a second command conditionally; - `$(...)` and backticks run a command and substitute its output; - `>` `<` redirect files; - `*` `?` glob filenames. If you hand the shell a *single string* that contains untrusted input, the shell will happily interpret any metacharacters in that input as **syntax**, not data — the same data-as-code confusion as SQLi. ## The broken pattern ```java // DANGEROUS: a single string handed to a shell String filename = request.getParameter("f"); Runtime.getRuntime().exec("sh -c \"convert " + filename + " out.png\""); ``` If `filename` is `a.jpg; rm -rf /`, the shell sees two commands and runs both. Even `Runtime.exec(String)` *without* an explicit shell is dangerous: its single-string overload splits the command naively on whitespace (it does **not** honor quoting), so it both breaks on legitimate spaces and can be abused. ## The fix: an argument vector, no shell The reliable defense is to **not invoke a shell** and to pass the program plus each argument as **separate elements of a list/array**. Then the OS executes the named program directly (`execvp`-style), handing it that exact argument vector. Nothing parses metacharacters. ```java ProcessBuilder pb = new ProcessBuilder( "/usr/bin/convert", // absolute path to the executable filename, // one element = one argument, verbatim "out.png"); pb.redirectErrorStream(true); Process p = pb.start(); ``` Now `filename = "a.jpg; rm -rf /"` is passed to `convert` as one literal argument named `a.jpg; rm -rf /` — `convert` just fails to find that file. The `;` is never special because no shell is in the picture. (`Runtime.exec(String[])` gives the same property; `ProcessBuilder` is the modern, more flexible API.) ## Additional hardening - **Use an absolute executable path** (`/usr/bin/convert`, not `convert`). Otherwise the program is resolved via the `PATH` environment variable, and an attacker who can influence `PATH` or drop a malicious binary earlier in it can hijack the call (**PATH hijacking**). - **Don't pass secrets as arguments.** Process arguments are visible system-wide (e.g. `ps`, `/proc`). Pass secrets via stdin or environment instead — and even environment has caveats. - **Validate file paths separately** for path traversal if an argument is a path (see canonical-path checks). - **If you truly need shell features** (pipes, globbing), you've reintroduced the danger; restrict inputs to a strict **allow-list** of known-safe values and/or escape rigorously — but prefer redesigning to avoid the shell. - **Least privilege:** run the child process as an unprivileged user so a successful injection is contained. ## Why this is the same idea as SQLi Both are **injection** flaws: untrusted data crosses a trust boundary into an interpreter (a shell / a SQL engine) that mixes code and data. The general cure is the same — **keep data out of the code channel**: bind parameters for SQL, pass an argument vector (no shell) for commands.