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 spider/php/crawler.py file acts as a bridge between a Python web crawler and a set of PHP spider scripts, using subprocess.run() to invoke PHP logic at runtime. A flaw in the PHPBridge.call() method — specifically at line 197 — meant that two externally influenced values, self.spider_path and method, were assembled into a subprocess command list with zero validation. This created a direct path for an attacker to execute arbitrary PHP files or manipulate method dispatch in ways the application never intended.

The Vulnerability Explained

Here is the vulnerable code as it existed before the fix:

def call(self, method, *args):
    # 构建命令
    cmd = [PHP_CMD, BRIDGE_SCRIPT, self.spider_path, method]
    cmd_args = []
    # ... rest of execution

Two values flow straight into the cmd list without any checks:

1. self.spider_path — set during __init__ from the spider_path argument, which can originate from CLI input or external configuration.
2. method — passed directly by the caller, potentially from a request handler or external data source.

Even though shell=False is correctly used (the array form of subprocess.run()), this only prevents the shell from interpreting metacharacters like ;, &&, or |. It does not prevent an attacker from substituting an entirely different file path.

The Attack Scenario

Consider what happens when an attacker controls spider_path via a CLI argument or a configuration value:

# Attacker supplies this as the spider path:
../../../tmp/malicious.php

The resulting command becomes:

cmd = ["php", "bridge.php", "../../../tmp/malicious.php", "parse"]

PHP dutifully executes ../../../tmp/malicious.php. If the attacker has previously written a PHP web shell or data-exfiltration script to a world-writable directory, they now have arbitrary code execution within the PHP runtime context.

Similarly, a method value like __destruct or a string containing Unicode look-alike characters could invoke unintended PHP class methods if the bridge script uses dynamic dispatch.

The PR's own exploitation scenario makes this concrete:

"An attacker who can control the spider file path (e.g., through the CLI argument or configuration) can provide a path to a malicious PHP script."

Because this is described as a web service, the spider_path and method values may ultimately trace back to HTTP request parameters — making this directly exploitable by remote, unauthenticated attackers with no prior access.

The Fix

The fix inserts two validation guards at the very top of PHPBridge.call(), before the command list is ever constructed:

def call(self, method, *args):
    # 安全校验: 限制 method 为合法标识符, spider_path 必须是存在的 .php 文件,
    # 避免外部输入被当作可执行脚本路径/方法名传入子进程
    if not isinstance(method, str) or not method.isidentifier():
        print(f"[Bridge Error] Invalid method name: {method}")
        return None
    if not (os.path.isfile(self.spider_path) and self.spider_path.lower().endswith('.php')):
        print(f"[Bridge Error] Invalid spider path: {self.spider_path}")
        return None

    # 构建命令
    cmd = [PHP_CMD, BRIDGE_SCRIPT, self.spider_path, method]
    cmd_args = []

Before vs. After

Why Each Check Matters

method.isidentifier() leverages Python's built-in lexer rules to confirm the string matches [a-zA-Z_][a-zA-Z0-9_]*. This is exactly the character set that constitutes a valid PHP method name, so it's both a security and a correctness constraint. Strings like "; rm -rf /", empty strings, or Unicode injection attempts all fail this check immediately.

os.path.isfile(self.spider_path) and self.spider_path.lower().endswith('.php') provides two layers of protection:
- os.path.isfile() resolves the path and confirms it exists as a regular file. A traversal path like ../../../etc/passwd that does exist would still pass this check — which is why the second condition matters.
- .endswith('.php') ensures only PHP files can be targeted. /etc/passwd, /tmp/evil.sh, and other non-PHP paths are rejected regardless of whether they exist.

Together, these two checks close the attack surface without changing any legitimate behavior: valid spider scripts are real .php files with method names that are legal identifiers.

Prevention & Best Practices

1. Validate before you delegate

Any time your Python code constructs a command list for subprocess, treat every element as potentially tainted. Ask: "Where does this value come from, and what are its valid values?" If the answer involves user input or configuration, add an explicit allowlist or format check.

2. Prefer allowlists over blocklists

The fix uses isidentifier() — an allowlist of valid characters — rather than trying to strip or reject specific dangerous characters. Blocklists are fragile; allowlists are robust.

3. Resolve and confine file paths

For file path arguments, consider going further than the current fix by resolving the absolute path and confirming it falls within an expected base directory:

import os

BASE_DIR = os.path.realpath("/app/spiders")
resolved = os.path.realpath(self.spider_path)

if not resolved.startswith(BASE_DIR + os.sep):
    raise ValueError(f"Path escapes spider directory: {self.spider_path}")

This prevents an attacker from supplying a symlink or a path that resolves outside the intended directory.

4. Use shell=False — but don't stop there

shell=False (the array form of subprocess.run()) is necessary but not sufficient. It prevents shell metacharacter injection but does nothing to prevent argument-level abuse, as this vulnerability demonstrates.

5. Log and monitor validation failures

The fix correctly logs [Bridge Error] messages when validation fails. In production, these should feed into a security monitoring system. A burst of Invalid spider path errors is a strong signal of active exploitation attempts.

Relevant Standards

• OWASP A03:2021 – Injection: Subprocess argument injection is a form of OS command injection.
• CWE-78: Improper Neutralization of Special Elements used in an OS Command.
• CWE-22: Path Traversal — the spider_path attack vector also qualifies.

Key Takeaways

• shell=False is not a complete defense: PHPBridge.call() already used the array form of subprocess.run(), yet was still vulnerable because the arguments themselves were attacker-controlled.
• str.isidentifier() is an underused security primitive: For any code that dispatches to named methods or functions via external input, Python's built-in isidentifier() is a clean, zero-dependency allowlist check.
• File path arguments need two checks: Existence (os.path.isfile) plus extension (.endswith('.php')) together prevent the most common path traversal attacks. Adding os.path.realpath() confinement makes this defense-in-depth.
• Web services amplify subprocess risk: Because this crawler runs as a web service, what might be a local-only risk in a CLI tool becomes a remotely exploitable vulnerability. Threat model context matters when prioritizing fixes.
• Validation must happen at the sink, not just the source: Even if the CLI or config layer validates spider_path, the call() method should re-validate because the value could arrive through other code paths.

How Orbis AppSec Detected This

• Source: The spider_path argument passed to PHPBridge.__init__() from CLI input or external configuration, and the method argument passed to PHPBridge.call() from request-handling code.
• Sink: The command list construction cmd = [PHP_CMD, BRIDGE_SCRIPT, self.spider_path, method] followed by subprocess.run() in spider/php/crawler.py at line 197.
• Missing control: No validation of method format and no verification that spider_path is a legitimate, existing .php file before the subprocess was invoked.
• CWE: CWE-78 — Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection'), with a secondary CWE-22 (Path Traversal) for the spider_path vector.
• Fix: Added method.isidentifier() and os.path.isfile() + .endswith('.php') guards at the top of PHPBridge.call() to reject any input that doesn't conform to expected formats before the command list is constructed.

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 PHPBridge.call() vulnerability is a textbook reminder that argument injection is just as dangerous as shell injection, and that shell=False alone is not a security guarantee. The path from an unvalidated spider_path to arbitrary PHP execution was short and direct. The fix — two concise validation checks using isidentifier() and os.path.isfile() — closes this path entirely while preserving all legitimate functionality.

For developers building similar Python-to-subprocess bridges: validate every argument against the tightest possible allowlist, resolve and confine file paths to expected directories, and treat validation failures as security events worth monitoring. These small habits prevent large incidents.

References

• CWE-78: Improper Neutralization of Special Elements used in an OS Command
• CWE-22: Improper Limitation of a Pathname to a Restricted Directory (Path Traversal)
• OWASP OS Command Injection Defense Cheat Sheet
• Python subprocess — Security Considerations
• Semgrep rules: subprocess injection
• fix: sanitize subprocess call in crawler.py