# Command Injection via subprocess.shell=True

Language: Python
Severity: Critical
CWE: CWE-78

## Source
3

## Flow
3-4-5, 3-7

## Sink
5, 7

## Vulnerable Code
```python
import subprocess

def provision_iot_device(device_mac, firmware_url):
    log_path = f"/var/log/iot/provision_{device_mac}.log"
    cmd = f"wget {firmware_url} -O /tmp/firmware.bin && sha256sum /tmp/firmware.bin >> {log_path}"
    result = subprocess.run(cmd, shell=True, capture_output=True, text=True)
    if result.returncode == 0:
        flash_cmd = f"iot-flasher --device {device_mac} --firmware /tmp/firmware.bin"
        subprocess.run(flash_cmd, shell=True)
        return {"status": "provisioned", "device": device_mac}
    return {"status": "failed", "error": result.stderr}
```

## Explanation

The function accepts untrusted input (device_mac and firmware_url) which is directly interpolated into shell commands executed via subprocess.run() with shell=True. An attacker can inject arbitrary shell commands through either parameter, leading to remote code execution.

## Remediation

The fix eliminates command injection by replacing all shell=True subprocess calls with argument lists (shell=False), which prevents shell metacharacter interpretation. Input validation is added to ensure device_mac conforms strictly to MAC address format and firmware_url uses only allowed schemes without shell-sensitive characters. File I/O for logging is handled via Python's built-in open() instead of shell redirection, and path traversal is prevented by resolving and validating the log path.

## Secure Code
```python
import subprocess
import re
import os
from urllib.parse import urlparse

def validate_mac_address(device_mac):
    """Validate that the input is a properly formatted MAC address."""
    mac_pattern = re.compile(r'^([0-9A-Fa-f]{2}[:-]){5}([0-9A-Fa-f]{2})$')
    if not mac_pattern.match(device_mac):
        raise ValueError(f"Invalid MAC address format: {device_mac}")
    return device_mac

def validate_firmware_url(firmware_url):
    """Validate that the firmware URL uses an allowed scheme and has a valid structure."""
    parsed = urlparse(firmware_url)
    if parsed.scheme not in ('http', 'https'):
        raise ValueError(f"Invalid URL scheme: {parsed.scheme}. Only http and https are allowed.")
    if not parsed.netloc:
        raise ValueError("Invalid URL: missing hostname.")
    if any(c in firmware_url for c in [';', '|', '&', '`', '$', '(', ')', '{', '}', '\n', '\r']):
        raise ValueError("URL contains disallowed characters.")
    return firmware_url

def provision_iot_device(device_mac, firmware_url):
    device_mac = validate_mac_address(device_mac)
    firmware_url = validate_firmware_url(firmware_url)

    firmware_path = "/tmp/firmware.bin"
    log_path = f"/var/log/iot/provision_{device_mac}.log"

    log_dir = os.path.dirname(log_path)
    resolved_log_path = os.path.realpath(log_path)
    if not resolved_log_path.startswith("/var/log/iot/"):
        raise ValueError("Invalid log path.")

    download_result = subprocess.run(
        ["wget", firmware_url, "-O", firmware_path],
        capture_output=True, text=True
    )

    if download_result.returncode != 0:
        return {"status": "failed", "error": download_result.stderr}

    hash_result = subprocess.run(
        ["sha256sum", firmware_path],
        capture_output=True, text=True
    )

    if hash_result.returncode == 0:
        with open(resolved_log_path, "a") as log_file:
            log_file.write(hash_result.stdout)

        flash_result = subprocess.run(
            ["iot-flasher", "--device", device_mac, "--firmware", firmware_path],
            capture_output=True, text=True
        )

        if flash_result.returncode == 0:
            return {"status": "provisioned", "device": device_mac}
        else:
            return {"status": "failed", "error": flash_result.stderr}

    return {"status": "failed", "error": hash_result.stderr}
```