security-audit

Files (1)

• SKILL.md

SKILL.md (first lines)

# Web Application Security Audit

An open-ended instruction to "find vulnerabilities" produces plausible findings, most of
them wrong and none demonstrated. Work these phases in order instead. Each check is narrow
enough to run without judgement and specific enough that a negative result means something.

Examples are Python/FastAPI because that is where these were catalogued; every pattern has a
direct equivalent in Express, Flask, Rails and Django. Items marked **Observed** are real
mistakes found in production code, including several made by the auditor while auditing.

Add these exclusions to every command, or results are unreadable:

```
--exclude-dir=node_modules --exclude-dir=.venv --exclude-dir=__pycache__
--exclude-dir=dist --exclude-dir=build --exclude-dir=.git
```

---

## Phase 0 — Map the surface before looking for bugs

Do not start grepping for bugs. Roughly half the findings fall out of the map.

```bash
grep -rnE '@(app|router)\.(get|post|put|patch|delete)\(' --include='*.py' .   # routes
grep -rnE "getenv|environ\[|process\.env" --include='*.py' --include='*.js' . # config
grep -rn "async def" --include='*.py' .                                       # async handlers
```

Answer five questions **in writing**:

| Question | What it exposes |
|---|---|
| What authenticates a request, and can the client influence it? | Header-spoofing auth bypass (1.1) |
| Does any credential travel in a request **body** rather than a header? | Replay, transport and logging exposure (1.4) |
| What is *ambient* authority — cookies, source IP, network position? | CSRF where `SameSite` does not apply (1.11) |
| Which endpoints change state or move money? | Serialisation (1.6) and CSRF (1.11) |
| What is written to logs, and who can influence its contents? | 1.2, 1.10, 1.13 |

For each route, record **whether the auth guard runs before any data lookup**. A guard after
the lookup is an enumeration oracle: `404` versus `401` reveals which identifiers exist.

---

## Phase 1 — The pattern checks

### 1.1 Client-supplied headers trusted for identity

```bash
grep -rnE "X-Forwarded-For|X-Real-IP|X-Forwarded-Proto|HTTP_X_FORWARDED" --include='*.py' .
```

For each hit: **is the direct peer verified as a known proxy first?** If not, attacker-
controlled input is being used as identity.

**Proof.** `curl -H 'X-Forwarded-For: <an allowlisted ip>' http://host/`

**False alarm.** Used only for display or logging. Worth fixing (log forgery), not a bypass.

**Fix.** Honour forwarded headers only from a configured trusted-proxy list, and take the
**rightmost** hop that is not itself a trusted proxy — proxies *append*, so the leftmost
value is the one the client chose.

**Check the framework layer too.** Many servers rewrite the client address from
`X-Forwarded-For` before your code runs (Uvicorn's `ProxyHeadersMiddleware` does this by
default for loopback peers). An application check is meaningless if the framework already
overwrote the value it inspects.

### 1.2 Secrets reaching logs

```bash
# Direct: a secret-named variable at a log call site
grep -rnE "(password|secret|token|api_key|passphrase|private_key)" --include='*.py' . \
  | grep -iE "logger\.|logging\.|print\(|console\."

# Indirect — THE ONE THAT GETS MISSED: a whole structure dumped
grep -rnE "(logger|logging|console)\.[a-z]+\(.*(json\.dumps|JSON\.stringify|repr|vars|__dict__|payload|body|config)" \
  --include='*.py' --include='*.js' .
```

> **Observed.** The first grep returned clean while `logger.info(f"payload: {json.dumps(data)}")`
> was writing an API key to disk — the secret was *inside a dict*, not named at the call
> site. Always run the second grep.

**Proof.** Capture log output during a request; assert the literal secret is absent. **Set
the capture level to DEBUG first** — at the default `WARNING` nothing is captured and the
absence assertion passes for the wrong reason.

**Check the notification path.** If ERROR-level logs are forwarded to chat/email/push, every
leaked secret is also transmitted off-host.

### 1.3 Secrets partially rendered in a UI

```bash
grep -rnE "\[:[0-9]+\].*\[-[0-9]+:\]|substring\([0-9]|slice\([0-9]" templates/ src/ views/
```

**Proof.** Render the page, then compute the longest substring of the real secret appearing
anywhere in the output. More than ~4 characters is too much.

**Subtlety.** Use a fixed-width mask: `abc...xyz` leaks the length, `••••••••1234` does not.

### 1.4 A bearer credential inside the request body

Common for webhooks. Implies **three** findings — check all:

| Consequence | Check | Fix |
|---|---|---|
| Replayable forever | Is the request required to be recent *and* unique? | Freshness window + idempotency key |
| Credential crosses the wire in cleartext | Is TLS enforced on that endpoint? | Refuse non-TLS delivery |
| Credential lands in logs | See 1.2 | Redact |

**Proof of replay.** Capture one valid request; send the exact bytes again.

**Sizing trap.** An idempotency entry must outlive the freshness of what it protects. If the
window accepts timestamps ±W, a request dated `+W` in the future is accepted now and stays
fresh until `W` past its own timestamp — up to **2W** after acceptance. A TTL of `W` leaves
it replayable in the gap. Size the cache at `2W`.

From cslev/skillet, MIT.

More from cslev/skillet