How command injection happens in Kotlin/Android and how to fix it

In V2rayNG, a root shell helper trusted string concatenation a little too much

In V2rayNG's Android codebase, we found a critical command injection vulnerability in RootShell.kt — the utility object responsible for running privileged commands via Android's su binary. The exec() method took a command: String and handed it straight to ProcessBuilder("su", "-c", command). Because su -c interprets its argument through a shell, any shell metacharacters inside that string get executed with root privileges. Worse, exec() was public, meaning any code — now or in the future — could call it with attacker-influenced data and trigger arbitrary command execution as root.

This matters a lot for VPN apps like V2rayNG that legitimately need root access for certain networking features. When root-level tooling is exposed through a loosely-typed string API, a single unescaped path, config value, or IPC message can turn a helper utility into a full device compromise vector.

The Vulnerability Explained

Here's the vulnerable code path before the fix:

fun runScript(script: String): Result {
    val file = File(...).apply {
        writeText(script)
        setExecutable(true, false)
    }
    return exec("sh ${file.absolutePath}")
}

fun exec(command: String, timeoutSeconds: Long = 30): Result {
    return try {
        val process = ProcessBuilder("su", "-c", command)
            .redirectErrorStream(true)
        ...

Two problems stack up here:

1. runScript() builds a command via raw string interpolation. file.absolutePath is dropped directly into "sh ${file.absolutePath}" with no quoting or escaping. If that path ever contained a space, a single quote, a semicolon, or backticks — whether from a customized data directory, a symlink, or a compromised storage location — the resulting string could be reinterpreted by the shell in unexpected ways.
2. exec() was public and accepted any string. Because ProcessBuilder("su", "-c", command) passes command to a shell, exec() was effectively a general-purpose "run anything as root" function. Any future caller — a settings screen, a config importer, an IPC handler — that passed user-influenced text into exec() would immediately create a full command injection primitive.

Example attack scenario: Imagine a future feature (or a supply-chain-compromised dependency) calls RootShell.exec(userSuppliedProfileName) to tag a log file. If userSuppliedProfileName were something like:

myprofile; rm -rf /data & 

su -c would happily execute myprofile, then rm -rf /data as root on the device. Even without a directly malicious caller, the runScript() path itself was building sh <path> without quoting — meaning a maliciously named or symlinked file path could inject shell syntax into a root-executed command.

The real-world impact: since this code runs with su, exploitation isn't limited to app-sandbox data — it's a path to root-level code execution, arbitrary file access, and full device compromise on rooted Android devices running V2rayNG.

The Fix

The PR makes two complementary changes to close both the exposure surface and the injection point:

1. Restrict the API surface — exec() is now private:

- fun exec(command: String, timeoutSeconds: Long = 30): Result {
+ private fun exec(command: String, timeoutSeconds: Long = 30): Result {

This is a critical containment step. By making exec() private, the module guarantees that raw shell commands built from arbitrary strings can never be invoked from outside RootShell.kt. Any future contributor who wants to run a root command must go through the safer, controlled entry points (runScript()), rather than being tempted to hand-build a command string elsewhere in the codebase and call exec() directly.

2. Escape the file path before it reaches the shell:

- return exec("sh ${file.absolutePath}")
+ val safePath = file.absolutePath.replace("'", "'\\''")
+ return exec("sh '$safePath'")

This applies the standard POSIX shell single-quote escaping technique: wrap the value in single quotes, and for any embedded single quote, replace it with '\'' (close the quote, insert an escaped literal quote, reopen the quote). Wrapping safePath in single quotes means the shell treats the entire path as one literal argument — spaces, semicolons, backticks, and $() inside the path are no longer interpreted as shell syntax, they're just characters in a string.

Together, these changes mean:
- The only string ever passed to su -c from this codebase is one that has been explicitly quoted and escaped.
- No external caller can bypass that protection by calling exec() directly with untrusted input, because exec() is no longer accessible outside the file.

Prevention & Best Practices

• Never build shell commands via string interpolation with dynamic values, even for "internal" data like file paths. File paths can be influenced indirectly through symlinks, renamed files, or misconfigured storage locations.
• Minimize the visibility of dangerous APIs. If a function ultimately executes a string through a shell (directly or via su -c, sh -c, bash -c), keep it private or internal and route all callers through safer, narrowly-scoped wrappers.
• Prefer argument arrays over shell strings when possible. ProcessBuilder(listOf("su", "-c", "some_fixed_binary", arg1, arg2)) with individual arguments avoids shell reinterpretation entirely — reserve su -c "<script>" only for cases where a full shell script genuinely needs to run, and quote/escape rigorously when you do.
• Apply proper shell escaping for any value embedded in a shell string. The single-quote-and-escape technique (' → '\'', wrapped in '...') used in this fix is the standard POSIX-safe approach.
• Use static analysis to catch this pattern early. Tools like Semgrep and CodeQL have rules specifically for ProcessBuilder/Runtime.exec() calls that embed unsanitized string concatenation — wire these into CI for any codebase that touches su, sh -c, or subprocess execution.
• Reference CWE-78 ("Improper Neutralization of Special Elements used in an OS Command") and the OWASP Command Injection guidance when reviewing any code that shells out.

Key Takeaways

• RootShell.exec() executed strings through su -c, meaning any unescaped shell metacharacter in the input became a root-privileged command — this is a textbook CWE-78 command injection.
• runScript() was vulnerable even without a malicious caller: it interpolated file.absolutePath directly into "sh ${file.absolutePath}" without quoting.
• Making exec() private closes off the injection surface for any future or external caller in the V2rayNG codebase.
• The fix uses standard POSIX single-quote escaping (' → '\'', wrapped in '...') to neutralize shell metacharacters in the file path before it reaches su -c.
• Root-level utilities like RootShell.kt deserve extra scrutiny — because the blast radius of a command injection here is full root access, not just app-sandbox compromise.

How Orbis AppSec Detected This

• Source: The command parameter of the public RootShell.exec() method, and the interpolated file.absolutePath value inside runScript().
• Sink: ProcessBuilder("su", "-c", command) in RootShell.kt:34, which executes the string through a root shell.
• Missing control: No shell escaping/quoting of dynamic values before command-string construction, and no access restriction preventing external callers from invoking exec() with arbitrary input.
• CWE: CWE-78 — Improper Neutralization of Special Elements used in an OS Command.
• Fix: exec() was made private, and runScript() now single-quote-escapes the file path before embedding it in the sh '<path>' command string.

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 vulnerability is a strong reminder that "internal" helper functions running with elevated privileges — like RootShell.exec() calling su -c — need to be treated as high-risk attack surface, not convenience utilities. A single unescaped string concatenation, "sh ${file.absolutePath}", was enough to turn a legitimate root-access feature into a potential root command injection vector. The fix combines two proven mitigations: restricting API visibility so untrusted callers can't reach the dangerous sink, and properly escaping dynamic values before they're embedded in a shell command. Any codebase that shells out with elevated privileges should apply both principles — least-privilege API design and rigorous input escaping — as standard practice.

References

• CWE-78: Improper Neutralization of Special Elements used in an OS Command
• OWASP OS Command Injection Defense Cheat Sheet
• Java ProcessBuilder Documentation
• Semgrep: command injection rules
• fix: sanitize shell/subprocess call in RootShell.kt