Skip to content

CWE-77: Command Injection - Java

Overview

Command injection in Java occurs when code builds a system command from untrusted input and hands it to Runtime.exec(String) or a ProcessBuilder wrapped in a shell (sh -c, cmd /c). The two behave differently, and the difference decides which payload works. Only the shell wrapper interprets metacharacters (;, &, |, backticks, $()), letting the input terminate the intended command and start a new one. Runtime.exec(String) runs no shell - it splits the string on whitespace - so the injected text becomes extra arguments to the program that was already going to run.

Primary defense: use a Java library for the task (networking, file I/O, compression) instead of shelling out. When a system command is unavoidable, use ProcessBuilder with a separate argument for each token - never Runtime.exec(String) or a shell wrapper - plus strict allowlist validation.

Common Vulnerable Patterns

A command assembled as one string, split on whitespace

// VULNERABLE - the input decides where the argument boundaries fall
String userIP = request.getParameter("ip");
Runtime.getRuntime().exec("ping " + userIP);
// Attack: ip = "-c 100000 8.8.8.8" -> the tokens become extra ping arguments
// Not an attack: ip = "8.8.8.8; cat /etc/passwd" -> no shell is involved, so
// ";", "cat" and "/etc/passwd" reach ping as three literal arguments

Why this is vulnerable: Runtime.exec(String) tokenizes the string with a StringTokenizer on whitespace and calls the String[] overload. There is no shell in the path, so the metacharacter payload above does nothing - which is why a finding on this line is often dismissed as a false positive. What the attacker actually controls is the argument list of a program that was always going to run: every whitespace-separated token in userIP becomes another argument to ping. That is enough to change what the command does, and it is full command execution wherever the invoked program accepts an option that runs something - tar --checkpoint-action=exec=, find -exec, rsync -e, git --upload-pack. Tokenization is also why the overload has been deprecated since Java 18: it decides the program/argument split from a string the caller assembled, and it has no concept of quoting, so a legitimate value containing a space silently becomes two arguments.

An explicit shell wrapper

// VULNERABLE - "/bin/sh", "-c" makes the next element a shell script, not an argument
String domain = request.getParameter("domain");
ProcessBuilder pb = new ProcessBuilder("/bin/sh", "-c", "nslookup " + domain);
pb.start();
// Attack: domain = "example.com && whoami"

Why this is vulnerable: ProcessBuilder with one array element per token is the recommended fix for the pattern above, and this code has that shape, which is what makes it easy to sign off on. The guarantee the array form gives is that each element arrives at the process as one literal argument - and here the element being delivered intact is an entire shell script. sh then parses it, so ;, &&, |, backticks and $() all apply to whatever domain contains. Escaping the metacharacters is not the fix, because there is no escaping rule that survives a maintainer adding one more concatenation; removing /bin/sh, -c is, so that the argument list is the process's own argument list. Note also that anything placed after the script string becomes $0, $1 and so on rather than a further command, so appending arguments does not separate the data from the script.

Secure Patterns

Use Java Native APIs (Primary Defense)

// Instead of: exec("ping " + host)
import java.net.InetAddress;
import java.util.regex.Pattern;

// No leading hyphen, and matches() rather than find() - see "Validate Input".
private static final Pattern HOSTNAME =
        Pattern.compile("[a-zA-Z0-9][a-zA-Z0-9.-]*");

public boolean isHostReachable(String host) throws IOException {
    if (!HOSTNAME.matcher(host).matches()) {
        throw new IllegalArgumentException("Invalid hostname");
    }
    return InetAddress.getByName(host).isReachable(5000);
}

// Instead of: exec("curl " + url)                    -> java.net.http.HttpClient
// Instead of: exec("tar -czf archive.tar.gz " + f)    -> Apache Commons Compress (TarArchiveOutputStream)
// Instead of: exec("convert " + file + " output.pdf") -> Apache PDFBox / iText

Why this works: InetAddress, HttpClient, and the compression/PDF libraries talk to the OS or the file format directly through managed Java APIs - there is no shell in the path, so shell metacharacters in the input have no special meaning.

The deeper reason to prefer this over escaping is that it removes the weakness rather than managing it. Escaping has to be correct at every call site, forever: one refactor back to Runtime.exec(String) with concatenation, one argument that skips the quoting helper, and the vulnerability returns. A native API has no shell to inject into, so there is no rule for a future maintainer to get wrong.

It also avoids inheriting vulnerabilities from the CLI tool itself. ImageTragick (CVE-2016-3714) let a crafted image reach ImageMagick's delegate handling and execute shell commands - the injection happened inside the tool, past any escaping the calling application did. Calling PDFBox in-process instead of shelling out removes that exposure entirely.

There is a practical payoff too: these APIs throw typed exceptions such as UnknownHostException rather than returning an exit code and a stderr string, so failures are harder to swallow by accident.

Use ProcessBuilder with a Separate Argument per Token (When a Command Is Unavoidable)

import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.concurrent.TimeUnit;
import java.util.regex.Pattern;

private static final Pattern IPV4 = Pattern.compile(
        "((25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)\\.){3}"
        + "(25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)");

public String securePing(String ipAddress) throws IOException, InterruptedException {
    if (!IPV4.matcher(ipAddress).matches()) {
        throw new IllegalArgumentException("Invalid IP address");
    }

    // Each argument is a separate array element - no shell, no string to parse
    ProcessBuilder pb = new ProcessBuilder("ping", "-c", "4", ipAddress);
    pb.redirectErrorStream(true);
    // Send output to a file rather than a pipe: nothing in this method drains a
    // pipe, and an undrained one blocks the child once the OS buffer fills.
    Path out = Files.createTempFile("ping", ".txt");
    pb.redirectOutput(out.toFile());

    Process process = pb.start();
    if (!process.waitFor(10, TimeUnit.SECONDS)) {
        // waitFor() after destroyForcibly(), or the child still holds the file
        // open and the delete below fails on Windows.
        process.destroyForcibly().waitFor();
        Files.deleteIfExists(out);
        throw new IOException("Command timed out");
    }
    try {
        return Files.readString(out);
    } finally {
        Files.deleteIfExists(out);
    }
}

Why this works: ProcessBuilder with one argument per array element invokes the executable directly via fork()/exec() (Linux) or CreateProcess() (Windows), bypassing the shell entirely. Because there is no shell parsing the argument list, ;, &, |, and $() inside ipAddress are passed through as literal characters in a single argument rather than being interpreted as command syntax. Runtime.exec(String[]) provides the same array-argument guarantee, but ProcessBuilder is preferred for the extra control (redirecting streams, setting a working directory, applying a timeout).

Two details in that example are load-bearing and easy to drop. matcher().matches() anchors the whole string, which is why no ^/$ appears in the pattern - $ in Java also matches before a final line terminator, so matches() is the form that cannot be fooled by "8.8.8.8\n". And the output goes to a file rather than the default pipe: with redirectErrorStream(true) and nothing reading the pipe, a child that produces more than the OS buffer holds blocks inside write(), waitFor runs out its ten seconds, and a legitimate command is killed. Draining the pipe inline instead - process.getInputStream().readAllBytes() before waitFor - trades that for the opposite failure: the read blocks until EOF, so a child that prints one line and then hangs never reaches the timeout below it. A file or a separate reader thread avoids both.

Validate Input (Defense in Depth)

Even with the shell bypassed, validate before use: allowlist patterns for hostnames, strict numeric formats for IPs and ports, and explicit rejection of .., /, \ in filenames to block path traversal. Compile Pattern objects once and reuse them across every command-building code path so the same rule applies everywhere, and always fail closed (throw, don't attempt to "clean" the input).

Reject a leading hyphen. [a-zA-Z0-9.-]+ includes -, so -debug passes it and new ProcessBuilder("nslookup", "-debug") runs an option rather than looking up a host. The array form is not failing here - it delivers the element exactly as given, hyphen and all. Anchor the first character to something that cannot introduce a flag ([a-zA-Z0-9][a-zA-Z0-9.-]*), and pass -- before the positional arguments where the command supports it. That is CWE-88, and it is the part ProcessBuilder does not close.

Match with matches(), never find(), and do not assume the anchors are what protect you. String.matches() and Matcher.matches() require the whole input to be consumed, so "evil.com\n".matches("^[a-zA-Z0-9.-]+$") is false on JDK 26. That is the whole-region rule doing the work, not the ^ and $: measured on the same JDK, Pattern.compile("^[a-zA-Z0-9.-]+$").matcher("evil.com\n").find() is true, because Java's $ does sit before a final line terminator and find() is content to stop there. Drop the anchors as well and find() accepts any input containing a single permitted character. Treat find() in a validator as the finding.

The same asymmetry decides whether a pattern survives being ported. Java is the forgiving end: ^[a-zA-Z0-9.-]+$ through re.match() in Python, Regex.IsMatch() in .NET or preg_match() in PHP all accept the trailing newline that matches() refuses. Carried out of this page it needs re.fullmatch(), \A...\z or the D modifier; carried in, it is safe.

Framework-Specific Guidance

Servlet/Spring MVC parameter binding does not sanitize for command injection - a @RequestParam or request.getParameter() value is still untrusted at the point it reaches Runtime.exec() or ProcessBuilder. Apply the same validation and array-argument rules regardless of which layer bound the value.

Considerations

The first question is whether a subprocess is needed at all. Most findings of this kind are a shell call standing in for a library the platform already ships - fetching a URL, unpacking an archive, resizing an image. Replacing the call removes the weakness rather than containing it, and usually removes error handling and portability problems with it. That is a rewrite, so weigh it against hardening the existing call; but a hardened subprocess still runs another program with your privileges, and the library does not.

The timeouts in the examples are placeholders for a decision you have to make. A subprocess with no bound can hang a request thread indefinitely, so one is needed - but the right value comes from what the command legitimately does. Too short and normal work fails under load; too long and an attacker who can influence the input has a cheap way to exhaust your workers. Bound the output as well as the time: a command that returns unbounded data to an in-memory buffer is a denial of service whether or not the arguments were validated.

An allowlist is only as good as its most permissive entry. Restricting which command may run is worth doing, but a permitted command that itself takes a path, a URL, or a format string moves the problem one level down rather than solving it. Prefer allowing a fixed set of complete invocations over allowing a program and validating its arguments separately.

Testing

  • Normal input: securePing("8.8.8.8") returns output rather than throwing - a pattern that rejects everything passes every assertion below, so the accept is the one that separates a fix from a broken validator.
  • Boundary input: an empty string, a 300-character hostname, and a filename containing .. are all rejected.
  • Anchoring: "8.8.8.8\n" is rejected. matches() gives this for free; the same pattern through find(), or ported to Python's re.match, does not.
  • Argument injection: "-debug" is rejected before it reaches nslookup. It contains no shell metacharacter, so none of the payloads below exercises it.
  • Output volume: a command producing more than a pipe buffer's worth of output completes rather than hitting the timeout. Test with a chatty command, not a one-line one - the deadlock only appears above the OS buffer.
  • Malicious input: 8.8.8.8; cat /etc/passwd, `whoami`, $(cat /etc/shadow), and && rm -rf /tmp/* are all rejected or treated as a single literal argument, never executed.

Common Pitfalls

  • Array-form ProcessBuilder with a concatenated flag argument: new ProcessBuilder("convert", "--output=" + userInput) uses array form and correctly avoids the shell, but the target program's own flag parser can still be tricked if userInput starts with - or contains an unexpected delimiter. Array-form execution closes shell injection (CWE-77); it does not by itself close argument injection (CWE-88) into the invoked program's own option parsing.
  • Runtime.exec(String command) single-string overload: This overload does not invoke a shell, but it splits the string into tokens with StringTokenizer on whitespace - it has no concept of quoting, so it cannot safely pass an argument that itself contains a space or handle quoted substrings the way a shell would. Switching only some call sites to ProcessBuilder/exec(String[]) while leaving others on the single-string overload is a partial fix.
  • Explicit shell wrapper reintroduced for convenience: After migrating to array-form ProcessBuilder, using new ProcessBuilder("/bin/sh", "-c", "cmd " + arg) for one call site (to get shell globbing or piping) reintroduces the exact metacharacter-interpretation risk the migration removed - the fix must extend to every call site, not just the ones that were easy to convert.

Additional Resources