# Command Injection via subprocess.shell

Language: Python
Severity: Critical
CWE: CWE-78

## Source
3

## Flow
3-4-5, 3-4-8

## Sink
5, 8

## Vulnerable Code
```python
import subprocess

def rotate_iot_device_logs(device_id, compression_level):
    log_path = f"/var/iot/devices/{device_id}/logs"
    archive_cmd = f"tar -czf /backup/{device_id}_logs.tar.gz --level={compression_level} {log_path}"
    result = subprocess.run(archive_cmd, shell=True, capture_output=True, text=True)
    if result.returncode == 0:
        cleanup_cmd = f"find {log_path} -type f -mtime +7 -delete"
        subprocess.run(cleanup_cmd, shell=True)
        return {"status": "success", "archive": f"{device_id}_logs.tar.gz"}
    return {"status": "failed", "error": result.stderr}
```

## Explanation

The function accepts untrusted user input (device_id and compression_level) and directly interpolates them into shell commands without sanitization. Using subprocess.run() with shell=True enables command injection, allowing attackers to execute arbitrary commands by injecting shell metacharacters in either parameter.

## Remediation

The fix eliminates command injection by replacing shell=True with argument lists (shell=False by default), validating device_id with a strict whitelist regex, validating compression_level as an integer within expected range, and using os.path.realpath() to prevent path traversal attacks.

## Secure Code
```python
import subprocess
import re
import os

def rotate_iot_device_logs(device_id, compression_level):
    # Validate device_id: only allow alphanumeric characters, hyphens, and underscores
    if not re.match(r'^[a-zA-Z0-9_\-]+$', device_id):
        return {"status": "failed", "error": "Invalid device_id format"}
    
    # Validate compression_level: must be an integer between 1 and 9
    try:
        compression_level = int(compression_level)
        if compression_level < 1 or compression_level > 9:
            raise ValueError("Compression level out of range")
    except (ValueError, TypeError):
        return {"status": "failed", "error": "Invalid compression level. Must be integer 1-9."}
    
    log_path = f"/var/iot/devices/{device_id}/logs"
    archive_path = f"/backup/{device_id}_logs.tar.gz"
    
    # Verify the log path exists and is within expected directory
    real_log_path = os.path.realpath(log_path)
    if not real_log_path.startswith("/var/iot/devices/"):
        return {"status": "failed", "error": "Invalid log path"}
    
    # Use subprocess with argument list (shell=False) to prevent command injection
    archive_cmd = [
        "tar", "-czf", archive_path,
        f"--level={compression_level}", real_log_path
    ]
    result = subprocess.run(archive_cmd, capture_output=True, text=True)
    
    if result.returncode == 0:
        cleanup_cmd = [
            "find", real_log_path, "-type", "f", "-mtime", "+7", "-delete"
        ]
        subprocess.run(cleanup_cmd)
        return {"status": "success", "archive": f"{device_id}_logs.tar.gz"}
    return {"status": "failed", "error": result.stderr}
```