Business: HIGH security issues ARE launch blockers — fix before Phase 2

2026-02-14 16:48:40 +00:00 · 2026-02-14 16:48:40 +00:00 · c6010f1b6a
commit c6010f1b6a
parent e5b8769f7c
5 changed files with 472 additions and 59 deletions
--- a/projects/business/memory/security-audit.md
+++ b/projects/business/memory/security-audit.md
@ -0,0 +1,285 @@
+# DocFast Security Audit
+
+**Date:** 2026-02-14
+**Target:** https://docfast.dev (167.235.156.214)
+**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