06cdbf5fe2
- sessions.md: Replace dangerous 'do NOT revert to mail.cloonar.com' with correction - decisions.md: Mark Postfix scaling strategy as superseded - security-audit.md: Update target IP to K3s LB - bugs.md already correct (BUG-078 note says no Postfix on K3s)
285 lines
9.6 KiB
Markdown
285 lines
9.6 KiB
Markdown
# DocFast Security Audit
|
||
|
||
**Date:** 2026-02-14
|
||
**Target:** https://docfast.dev (was 167.235.156.214, now K3s LB 46.225.37.135)
|
||
**Auditor:** Automated Security Subagent
|
||
|
||
---
|
||
|
||
## CRITICAL Findings
|
||
|
||
### 1. CRITICAL — Stripe Webhook Signature Bypass
|
||
|
||
**File:** `src/routes/billing.ts` lines 75-85
|
||
|
||
The webhook handler has a conditional signature check:
|
||
```typescript
|
||
if (webhookSecret && sig) {
|
||
// verify signature
|
||
} else {
|
||
event = req.body as Stripe.Event; // ← ACCEPTS UNVERIFIED!
|
||
}
|
||
```
|
||
|
||
If `STRIPE_WEBHOOK_SECRET` is not set OR if the `stripe-signature` header is omitted, **any forged webhook is accepted**. This was confirmed live:
|
||
|
||
```
|
||
curl -X POST https://docfast.dev/v1/billing/webhook \
|
||
-H 'Content-Type: application/json' \
|
||
-d '{"type":"customer.subscription.deleted","data":{"object":{"customer":"cus_fake"}}}'
|
||
→ {"received":true}
|
||
```
|
||
|
||
**Impact:** An attacker can revoke any customer's API key by sending a forged `customer.subscription.deleted` event with their Stripe customer ID. Could also potentially provision Pro keys if checkout.session.completed handling is added later.
|
||
|
||
**Fix:** Always require signature verification. Remove the `else` fallback:
|
||
```typescript
|
||
if (!webhookSecret || !sig) {
|
||
res.status(400).json({ error: "Webhook not configured" });
|
||
return;
|
||
}
|
||
```
|
||
|
||
---
|
||
|
||
### 2. CRITICAL — SSRF via URL-to-PDF Endpoint
|
||
|
||
**File:** `src/routes/convert.ts` (URL endpoint) + `src/services/browser.ts`
|
||
|
||
The `/v1/convert/url` endpoint only checks that the protocol is `http:` or `https:` but does **not block private/internal IPs**. Puppeteer will navigate to any URL including:
|
||
|
||
- `http://127.0.0.1:3100/` (the app itself)
|
||
- `http://169.254.169.254/latest/meta-data/` (cloud metadata — Hetzner)
|
||
- `http://10.x.x.x/`, `http://172.16.x.x/` (internal networks)
|
||
- `file://` is blocked by protocol check, but DNS rebinding could bypass IP checks
|
||
|
||
**Impact:** An authenticated attacker (even with a free key) can scan internal services, access cloud metadata endpoints, and potentially exfiltrate secrets.
|
||
|
||
**Fix:** Add IP validation after DNS resolution:
|
||
```typescript
|
||
import { lookup } from "dns/promises";
|
||
|
||
async function isPrivateUrl(urlStr: string): Promise<boolean> {
|
||
const parsed = new URL(urlStr);
|
||
const { address } = await lookup(parsed.hostname);
|
||
const parts = address.split(".").map(Number);
|
||
return (
|
||
address === "127.0.0.1" ||
|
||
address.startsWith("::") ||
|
||
parts[0] === 10 ||
|
||
(parts[0] === 172 && parts[1] >= 16 && parts[1] <= 31) ||
|
||
(parts[0] === 192 && parts[1] === 168) ||
|
||
(parts[0] === 169 && parts[1] === 254) ||
|
||
parts[0] === 0
|
||
);
|
||
}
|
||
```
|
||
|
||
Also consider using `--disable-background-networking` and `--host-resolver-rules` in Puppeteer args to block at the browser level.
|
||
|
||
---
|
||
|
||
### 3. CRITICAL — XSS in Billing Success Page
|
||
|
||
**File:** `src/routes/billing.ts` (success endpoint)
|
||
|
||
The Pro API key is injected directly into HTML via template literal without escaping:
|
||
```typescript
|
||
res.send(`... onclick="navigator.clipboard.writeText('${keyInfo.key}')" ...>${keyInfo.key}</div> ...`);
|
||
```
|
||
|
||
While the key is server-generated (hex), the `email` field (from Stripe) is also used in key creation. If Stripe returns a malicious email or if the key format ever changes, this becomes exploitable. More importantly, the `session_id` query parameter is passed directly to Stripe's API — if Stripe returns unexpected data, it could inject HTML.
|
||
|
||
**Severity context:** Currently low exploitability since keys are hex-only, but it's a bad pattern.
|
||
|
||
**Fix:** Use proper HTML escaping for all dynamic values, or return JSON and render client-side.
|
||
|
||
---
|
||
|
||
## HIGH Findings
|
||
|
||
### 4. HIGH — Container Runs as Root with --no-sandbox
|
||
|
||
**File:** `Dockerfile` + `src/services/browser.ts`
|
||
|
||
- Container user: `root` (confirmed via `docker exec ... id`)
|
||
- Chromium launched with `--no-sandbox`
|
||
|
||
If a Chromium exploit is triggered (e.g., via malicious HTML in the convert endpoint), the attacker gets root inside the container. Combined with SSRF, this is dangerous.
|
||
|
||
**Fix:**
|
||
```dockerfile
|
||
RUN groupadd -r docfast && useradd -r -g docfast -d /app docfast
|
||
RUN chown -R docfast:docfast /app
|
||
USER docfast
|
||
```
|
||
And remove `--no-sandbox` (or use `--sandbox` with proper user namespaces).
|
||
|
||
---
|
||
|
||
### 5. HIGH — No Firewall Active
|
||
|
||
UFW is **inactive**. Iptables INPUT policy is **ACCEPT**. Exposed services:
|
||
- Port 22 (SSH)
|
||
- Port 80/443 (nginx)
|
||
- Port 631 (CUPS — printing service, unnecessary on a server!)
|
||
|
||
**Fix:**
|
||
```bash
|
||
ufw default deny incoming
|
||
ufw allow 22/tcp
|
||
ufw allow 80/tcp
|
||
ufw allow 443/tcp
|
||
ufw enable
|
||
# Also: apt remove cups
|
||
```
|
||
|
||
---
|
||
|
||
### 6. HIGH — SSH Root Login with Default Config
|
||
|
||
All SSH security settings are commented out (defaults):
|
||
- `PermitRootLogin` defaults to `prohibit-password` (key-only, acceptable but not ideal)
|
||
- `PasswordAuthentication` defaults to `yes` — **password brute force possible**
|
||
|
||
**Fix:**
|
||
```
|
||
PermitRootLogin no # Create a non-root user first
|
||
PasswordAuthentication no
|
||
```
|
||
|
||
---
|
||
|
||
### 7. HIGH — Unlimited Free Key Signup (Abuse Vector)
|
||
|
||
`POST /v1/signup/free` has no rate limiting beyond the global 100/min limit. An attacker can:
|
||
- Create unlimited free keys with different emails
|
||
- Each key gets 100 PDFs/month
|
||
- Script: 100 signups × 100 PDFs = 10,000 PDFs/month for free
|
||
|
||
**Fix:** Add per-IP rate limiting on signup (e.g., 3/hour), require email verification, or add CAPTCHA.
|
||
|
||
---
|
||
|
||
### 8. HIGH — CORS Wildcard on All Routes
|
||
|
||
```
|
||
Access-Control-Allow-Origin: *
|
||
```
|
||
|
||
This is set on **all routes**, including authenticated API endpoints. While the API uses `X-API-Key` header (which is listed in `Access-Control-Allow-Headers`), a malicious website could make cross-origin requests to the API if a user has their API key in a browser extension or similar context.
|
||
|
||
For a public API this is somewhat expected, but the wildcard combined with `Authorization` header exposure is risky.
|
||
|
||
**Fix:** For the landing page/docs: keep `*`. For API routes: either keep `*` (public API pattern) or restrict to known origins. At minimum, do NOT include `Authorization` in allowed headers from `*` origin — use credentialed CORS with specific origins instead.
|
||
|
||
---
|
||
|
||
## MEDIUM Findings
|
||
|
||
### 9. MEDIUM — Usage Tracking is In-Memory Only
|
||
|
||
**File:** `src/middleware/usage.ts`
|
||
|
||
Usage counts are stored in a `Map` in memory. On container restart, all usage resets to 0. Free tier users effectively get unlimited PDFs by waiting for restarts (or triggering them via DoS).
|
||
|
||
**Fix:** Persist usage to the `/app/data` volume (already mounted). Use the same JSON file approach as keys.
|
||
|
||
---
|
||
|
||
### 10. MEDIUM — No Concurrent PDF Limit (DoS Risk)
|
||
|
||
PDF generation uses Puppeteer with a shared browser instance. There's no limit on concurrent PDF generations. An attacker with a valid key could:
|
||
- Send 50+ concurrent requests
|
||
- Each opens a Chromium tab
|
||
- Exhausts the 512MB memory limit → OOM kill
|
||
|
||
The container has `mem_limit: 512m` and `cpus: 1.0` which provides some protection, but there's no application-level queuing.
|
||
|
||
**Fix:** Add a concurrency semaphore (e.g., max 5 concurrent PDF renders). Queue additional requests.
|
||
|
||
---
|
||
|
||
### 11. MEDIUM — Nginx Server Version Disclosed
|
||
|
||
```
|
||
Server: nginx/1.24.0 (Ubuntu)
|
||
```
|
||
|
||
**Fix:** Add `server_tokens off;` to nginx config.
|
||
|
||
---
|
||
|
||
### 12. MEDIUM — No Page Timeout/Resource Limits in HTML Convert
|
||
|
||
The `renderPdf` function uses `waitUntil: "networkidle0"` with a 15s timeout for HTML content. A malicious payload could:
|
||
- Include `<script>` that fetches external resources slowly
|
||
- Load large images/fonts
|
||
- Execute infinite loops (though Puppeteer timeout helps)
|
||
|
||
**Fix:** Disable JavaScript execution for HTML-to-PDF, or use `waitUntil: "domcontentloaded"`. Add `page.setJavaScriptEnabled(false)` for the HTML endpoint.
|
||
|
||
---
|
||
|
||
### 13. MEDIUM — API Key Exposed in Success Page URL
|
||
|
||
The billing success flow shows the API key in a rendered HTML page. The `session_id` is in the URL query string, which means:
|
||
- Browser history contains the session ID
|
||
- Referrer headers could leak it
|
||
- Anyone with the session ID can re-retrieve the key
|
||
|
||
**Fix:** Make the success endpoint one-time-use (track consumed session IDs).
|
||
|
||
---
|
||
|
||
## LOW Findings
|
||
|
||
### 14. LOW — No Email Verification on Signup
|
||
|
||
Free keys are issued instantly without email verification. This means:
|
||
- No way to contact users
|
||
- Fake emails accepted
|
||
- Can't enforce one-key-per-person
|
||
|
||
### 15. LOW — GDPR Considerations
|
||
|
||
Data stored in `/app/data/keys.json`:
|
||
- Email addresses
|
||
- API keys
|
||
- Stripe customer IDs
|
||
- Timestamps
|
||
|
||
No privacy policy, no data deletion endpoint, no consent mechanism. For EU launch, need:
|
||
- Privacy policy page
|
||
- Right to deletion (API or email-based)
|
||
- Data processing disclosure
|
||
|
||
### 16. LOW — No Request Logging/Audit Trail
|
||
|
||
No access logs beyond nginx. No record of which API key accessed what. Makes abuse investigation difficult.
|
||
|
||
### 17. LOW — Puppeteer `--no-sandbox` + `--disable-setuid-sandbox`
|
||
|
||
While these flags are common in Docker, they disable Chromium's security sandbox. Combined with running as root (#4), this amplifies any browser exploit.
|
||
|
||
---
|
||
|
||
## Summary
|
||
|
||
| Severity | Count | Key Issues |
|
||
|----------|-------|-----------|
|
||
| CRITICAL | 3 | Webhook forgery, SSRF, XSS pattern |
|
||
| HIGH | 5 | Root container, no firewall, SSH config, signup abuse, CORS |
|
||
| MEDIUM | 5 | In-memory usage, DoS, nginx version, no JS disable, session reuse |
|
||
| LOW | 4 | No email verify, GDPR, no audit log, sandbox flags |
|
||
|
||
## Priority Fix Order
|
||
|
||
1. **Stripe webhook** — Fix the signature bypass immediately (one-line change)
|
||
2. **SSRF** — Add private IP blocking to URL endpoint
|
||
3. **Firewall** — Enable UFW, remove CUPS
|
||
4. **SSH** — Disable password auth
|
||
5. **Dockerfile** — Add non-root user
|
||
6. **Signup rate limit** — Prevent free tier abuse
|
||
7. **Usage persistence** — Store to disk
|
||
8. **PDF concurrency limit** — Prevent DoS
|