# Insecure Subprocess Command Injection via Shell=True with Unsanitized Environment Variable Expansion

Language: Python
Severity: Critical
CWE: CWE-78

## Source
9

## Flow
9-11

## Sink
11

## Vulnerable Code
```python
import subprocess
import os
def fetch_iot_device_metrics(device_id, metric_type):
    os.environ['DEVICE_TARGET'] = device_id
    os.environ['METRIC_KEY'] = metric_type
    cmd_template = 'iot-cli poll --target=$DEVICE_TARGET --metric=$METRIC_KEY --format=json'
    proc = subprocess.run(cmd_template, shell=True, capture_output=True, text=True, env=os.environ.copy())
    if proc.returncode != 0:
        raise RuntimeError(f'Polling failed: {proc.stderr}')
    return proc.stdout
```

## Explanation

The function accepts unsanitized user-controlled inputs `device_id` and `metric_type` (lines 3-4), stores them in environment variables `DEVICE_TARGET` and `METRIC_KEY` (lines 4-5), then passes them into a shell command string with `shell=True` (line 7). Because the shell expands `$DEVICE_TARGET` and `$METRIC_KEY` at runtime, an attacker can embed shell metacharacters (e.g., `;`, `$()`, `&&`) inside those values to inject arbitrary commands that the shell will execute.

## Remediation

The fix eliminates shell=True by passing the command as a list of explicit arguments, so no shell expansion or metacharacter interpretation occurs. It also removes the mutation of os.environ (which was the injection vector) and adds strict input validation via allowlist regex to reject any device_id or metric_type containing shell metacharacters or unexpected characters.

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

def fetch_iot_device_metrics(device_id, metric_type):
    # Validate device_id: allow only alphanumeric characters, hyphens, and underscores
    if not re.fullmatch(r'[A-Za-z0-9_\-]+', device_id):
        raise ValueError(f'Invalid device_id: {device_id!r}')
    # Validate metric_type: allow only alphanumeric characters, hyphens, and underscores
    if not re.fullmatch(r'[A-Za-z0-9_\-]+', metric_type):
        raise ValueError(f'Invalid metric_type: {metric_type!r}')

    cmd = [
        'iot-cli', 'poll',
        f'--target={device_id}',
        f'--metric={metric_type}',
        '--format=json'
    ]
    proc = subprocess.run(cmd, capture_output=True, text=True)
    if proc.returncode != 0:
        raise RuntimeError(f'Polling failed: {proc.stderr}')
    return proc.stdout
```