webapp-testing Server Script: Shell Injection via Unquoted Command

The Vulnerability Explained

A critical command injection vulnerability existed in the web server testing utility script, which accepted arbitrary server startup commands via command-line arguments and executed them using Python's subprocess.Popen() with shell=True. This combination created a direct path for shell metacharacter injection.

The vulnerable code pattern looked like this:

process = subprocess.Popen(
    server['cmd'],
    shell=True,
    stdout=subprocess.PIPE,
    stderr=subprocess.PIPE
)

The server['cmd'] string originated from user-controlled input (command-line arguments parsed by argparse.REMAINDER), and was passed directly to a shell for interpretation. An attacker who could influence the command string—for example, by crafting a malicious argument like echo test; id—would have both commands executed:

# User supplies:
--server "echo test; id"

# Shell interprets as:
echo test
id

The shell metacharacters are evaluated before the process runs, meaning the attacker's injected command executes with the full privileges of the script process. More sophisticated payloads using $() syntax or backticks allow arbitrary command substitution:

# Attacker provides:
--server "backup.sh $(rm -rf /important/data)"

# The substitution happens in the shell, before backup.sh runs

For a web server testing context, this is particularly dangerous because the script typically runs in an environment with elevated privileges or privileged file access, making it an attractive target for privilege escalation or lateral movement.

Affected Versions

The Fix

The fix removes shell interpretation entirely and replaces it with safe argument tokenization using Python's shlex module. Here's the before-and-after:

Before (vulnerable):

process = subprocess.Popen(
    server['cmd'],
    shell=True,
    stdout=subprocess.PIPE,
    stderr=subprocess.PIPE
)

After (fixed):

import shlex

cmd = server['cmd']
cwd = None
if cmd.startswith('cd ') and ' && ' in cmd:
    cd_part, _, cmd = cmd.partition(' && ')
    cwd = cd_part[3:].strip()

process = subprocess.Popen(
    shlex.split(cmd),
    cwd=cwd,
    stdout=subprocess.PIPE,
    stderr=subprocess.PIPE
)

Three key changes work together to eliminate the vulnerability:

1. Remove shell=True: The process is now executed directly (via execve() on Unix systems) without a shell intermediary, so metacharacters like ; and $() are treated as literal argument characters, not shell syntax.

2. Tokenize with shlex.split(): This safely breaks the command string into individual arguments respecting shell quoting rules. If a user legitimately needs an argument containing spaces, they can quote it in the command line, and shlex.split() will preserve it as a single token—without shell interpretation of special characters.

3. Preserve cd DIR && CMD via cwd: The documented workflow of changing directories before running a command is explicitly supported by detecting the pattern and using subprocess's cwd parameter. This maintains backward compatibility without requiring shell involvement.

With these changes, an attacker's attempt to inject echo test; id results in shlex.split() producing a single argument ["echo", "test;", "id"], which the subprocess receives literally. The semicolon is not special—it's just a character in the third argument.

How Orbis AppSec Detected This

Source: Command arguments accepted via argparse.REMAINDER and passed to the server command field.

Sink: subprocess.Popen() invoked with the user-supplied command string and shell=True.

Missing control: No validation, escaping, or sanitization of the command string before passing it to the subprocess module. The use of shell=True meant the string was always interpreted by a shell, which is inherently unsafe for untrusted input.

CWE: CWE-78 – Improper Neutralization of Special Elements used in an OS Command.

Fix: Replace shell-based execution with direct process invocation via shlex.split() for tokenization, and explicit handling of directory-change workflows via the cwd parameter.

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.

Key Takeaways

• Never use shell=True with user-controlled input. The shell is a separate interpreter with its own syntax for special characters; passing untrusted data to it is equivalent to passing code to eval(). Even if you think you've sanitized the input, the shell's parsing rules may surprise you.

• shlex.split() is safe only when shell=False. The module tokenizes according to shell quoting rules but does not invoke a shell. Pairing it with shell=False (the default) keeps the benefits of user-friendly quoting without the code execution risk.

• Legitimate shell features (cd, &&, pipes) should be explicit, not implicit. If your application needs to support directory changes, pass cwd as a parameter. If it needs conditional execution, implement that in Python logic—not by concatenating strings and hoping they survive shell parsing.

• argparse.REMAINDER captures raw user input. Any value it provides should be treated as untrusted, even if it comes from the command line of a "trusted" administrator. Privilege escalation often starts with an admin script that accepts arguments without proper validation.

Conclusion

This vulnerability demonstrates a common but critical mistake: assuming that because a script is run locally or by an administrator, its inputs are somehow safe. The reality is that shell metacharacter injection is trivial to exploit, and the fix—removing the shell layer and tokenizing safely—is simple and adds no complexity to legitimate use cases.

By switching to direct process execution with shlex.split() and handling special workflows (like directory changes) through subprocess parameters rather than shell syntax, the script is now resilient to injection attacks while remaining fully functional for all documented workflows.