# http2pic Security Hardening & Bug Fixes

**Date:** 2026-04-20
**Status:** Approved

## Summary

Harden http2pic against abuse and fix two correctness bugs. All changes target `web/index.php`, `src/helpers.php`, `docker/start.sh`, and `docker-compose.yml`. No new dependencies.

---

## 1. Config & Environment

Two new optional env vars. Both default to off so existing deployments are unaffected.

| Var | Default | Effect |
|-----|---------|--------|
| `API_KEY` | `""` | If non-empty, all `/api` requests must supply it |
| `BLOCK_PRIVATE_IPS` | `false` | If `true`, rejects URLs that resolve to private/loopback/metadata IPs |

`docker/start.sh` writes both into `src/config.inc.php` alongside the existing `URL` define:

```php
define('API_KEY', '${API_KEY:-}');
define('BLOCK_PRIVATE_IPS', ${BLOCK_PRIVATE_IPS:-false});
```

`docker-compose.yml` gets commented-out examples:

```yaml
environment:
  - URL=http://localhost:8080
  # - API_KEY=your-secret-key       # if set, all /api requests must provide it
  # - BLOCK_PRIVATE_IPS=true        # block LAN/loopback/metadata IPs (safe default for public hosting)
```

---

## 2. API Key Authentication

Checked at the top of `case 'api':`, before any Chrome work.

**Logic:** if `API_KEY` is defined and non-empty, require the key. Accept it from either:
- HTTP header: `X-API-Key: <key>`
- Query param: `?key=<key>`

Header takes priority. Returns `401 Unauthorized` if missing or wrong.

```php
if (defined('API_KEY') && API_KEY !== '') {
    $provided = $_SERVER['HTTP_X_API_KEY'] ?? $_REQUEST['key'] ?? '';
    if ($provided !== API_KEY) {
        header('HTTP/1.0 401 Unauthorized');
        echo 'Invalid or missing API key';
        exit;
    }
}
```

**Usage examples (to add to README):**

```bash
# via header (preferred — not logged in access logs)
curl -H "X-API-Key: secret" "http://host/api?url=https://example.com"

# via query param (simpler for browser use)
curl "http://host/api?key=secret&url=https://example.com"
```

---

## 3. SSRF Protection (opt-in)

Active only when `BLOCK_PRIVATE_IPS=true`. Runs after URL format validation, before spawning Chrome.

**Flow:**
1. Extract hostname from target URL via `parse_url()`
2. Resolve to IP via `gethostbyname()`
3. If resolution fails (returns same string) or IP is private → `403 Forbidden`

**Blocked ranges:** `127.0.0.0/8`, `10.0.0.0/8`, `172.16.0.0/12`, `192.168.0.0/16`, `169.254.0.0/16` (AWS/cloud metadata).

Implementation uses `ip2long()` + bitmask checks — no regex.

A new `isPrivateIP(string $ip): bool` helper lives in `src/helpers.php`.

Returns `403 Forbidden` with body `URL not allowed` (no detail about why, to avoid enumeration).

---

## 4. Bug Fixes

### 4a. RemoteWebDriver request timeout (60ms → 60s)

```php
// before
RemoteWebDriver::create($serverUrl, $capabilities, 30000, 60);
// after
RemoteWebDriver::create($serverUrl, $capabilities, 30000, 60000);
```

4th param is milliseconds. `60` = 60ms, effectively zero. Fix to `60000`.

### 4b. Viewport set before page load

Move `window()->setSize()` to before `driver->get()` so the page loads at the correct viewport from the start. This ensures responsive layouts and media queries fire at the intended dimensions.

---

## 5. Security Hardening (always applied)

### 5a. Viewport dimension cap

After format validation, reject dimensions exceeding 3840×2160:

```php
$parts = explode('x', $viewport);
if ((int)$parts[0] > 3840 || (int)$parts[1] > 2160) {
    header('HTTP/1.0 400 Bad Request');
    echo 'Viewport exceeds maximum (3840x2160)';
    exit;
}
```

Prevents memory exhaustion via `99999x99999` requests.

### 5b. Generic error responses

Catch block logs full exception message but returns only:

```
Screenshot failed
```

Stops ChromeDriver internals (internal hostnames, paths, stack traces) leaking to clients.

### 5c. Log injection fix

`addToLog()` strips `\n`, `\r`, `\t` from data before writing:

```php
$data = str_replace(["\n", "\r", "\t"], ' ', $data);
```

Prevents crafted URLs/IPs from injecting fake log entries.

### 5d. getUserIP() fix

Drop `HTTP_CLIENT_IP` (client-controlled, untrustworthy). Take only the first IP from `X-Forwarded-For` (Caddy appends subsequent hops; first entry is the real client):

```php
function getUserIP() {
    if (!empty($_SERVER['HTTP_X_FORWARDED_FOR']))
        return trim(explode(',', $_SERVER['HTTP_X_FORWARDED_FOR'])[0]);
    return $_SERVER['REMOTE_ADDR'];
}
```

---

## Files Changed

| File | Changes |
|------|---------|
| `web/index.php` | API key check, SSRF check, viewport cap, viewport-before-get, timeout fix, generic errors |
| `src/helpers.php` | `isPrivateIP()` helper, log injection fix, `getUserIP()` fix |
| `docker/start.sh` | Write `API_KEY` and `BLOCK_PRIVATE_IPS` into config |
| `docker-compose.yml` | Commented env var examples |
| `src/config.inc.php` | (generated) gets two new defines |