170 lines
4.8 KiB
Markdown
170 lines
4.8 KiB
Markdown
# 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 |
|