# Command Injection via subprocess.run(shell=True)

Language: Python
Severity: Critical
CWE: CWE-78

## Source
3, 4

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

## Sink
5, 7

## Vulnerable Code
```python
import subprocess

def provision_iot_device(device_mac, firmware_url):
    sanitized_mac = device_mac.replace(':', '')
    deployment_cmd = f"curl -s {firmware_url} | tee /var/iot/firmware_{sanitized_mac}.bin && sha256sum /var/iot/firmware_{sanitized_mac}.bin"
    result = subprocess.run(deployment_cmd, shell=True, capture_output=True, text=True)
    if result.returncode == 0:
        verification_hash = result.stdout.split()[0]
        subprocess.run(f"logger 'IoT device {device_mac} provisioned with hash {verification_hash}'", shell=True)
        return {"status": "provisioned", "checksum": verification_hash}
    return {"status": "failed", "error": result.stderr}
```

## Explanation

The function accepts untrusted input (device_mac and firmware_url) and directly interpolates them into shell commands executed via subprocess.run(shell=True). Although device_mac has minimal sanitization (colon removal), both parameters can be exploited to inject arbitrary shell commands through metacharacters like semicolons, backticks, or pipe operators.

## Remediation

The fix eliminates all shell=True usage by using subprocess with argument lists for curl, replacing sha256sum with Python's hashlib for checksum computation, and replacing the shell logger command with Python's logging module. Input validation is added to strictly verify the MAC address format and firmware URL before processing.

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

logger = logging.getLogger(__name__)

def provision_iot_device(device_mac, firmware_url):
    # Validate MAC address format strictly
    if not re.match(r'^([0-9A-Fa-f]{2}:){5}[0-9A-Fa-f]{2}$', device_mac):
        return {"status": "failed", "error": "Invalid MAC address format"}

    # Validate firmware URL
    parsed_url = urlparse(firmware_url)
    if parsed_url.scheme not in ('http', 'https'):
        return {"status": "failed", "error": "Invalid firmware URL scheme"}
    if not parsed_url.hostname:
        return {"status": "failed", "error": "Invalid firmware URL"}

    sanitized_mac = device_mac.replace(':', '')
    firmware_path = os.path.join('/var/iot', f'firmware_{sanitized_mac}.bin')

    # Download firmware using subprocess without shell=True
    curl_result = subprocess.run(
        ['curl', '-s', '-o', firmware_path, '--', firmware_url],
        capture_output=True, text=True
    )
    if curl_result.returncode != 0:
        return {"status": "failed", "error": curl_result.stderr}

    # Compute SHA256 checksum using Python's hashlib instead of shell command
    sha256_hash = hashlib.sha256()
    try:
        with open(firmware_path, 'rb') as f:
            for chunk in iter(lambda: f.read(8192), b''):
                sha256_hash.update(chunk)
    except IOError as e:
        return {"status": "failed", "error": str(e)}

    verification_hash = sha256_hash.hexdigest()

    # Use Python's logging instead of shell logger command
    logger.info(f'IoT device {device_mac} provisioned with hash {verification_hash}')

    return {"status": "provisioned", "checksum": verification_hash}
```
