Files
Epicure/SECURITY_AUDIT.md
T
Arnaud 5a9e306357 fix: close SSRF/rebinding, IDOR, and stale-session authz gaps found in audit
Bump to 0.5.1. Fixes: unfollowed-redirect SSRF + DNS-rebinding in AI
url-import and webhook dispatch (new safeFetch with IP-pinned undici
dispatcher); cross-user photo deletion via unvalidated recipe/review
storage keys; comment-moderation and tier-quota checks trusting a
stale cached session role/tier instead of the DB.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2026-07-12 19:05:20 +02:00

9.0 KiB

Security Audit — Epicure

Date: 2026-07-12 Scope: full app (apps/web, packages/db), 3 independent rounds, each own lens. Bar: confidence ≥8/10 only.


Round 1 — Auth & Authorization

Finding 1: Stale cached role trusted for comment moderation — MEDIUM

  • File: apps/web/app/api/v1/comments/[id]/route.ts:33
  • Confidence: 8
  • Description: Route checks session.user.role === "user" directly. Role comes from Better Auth cookieCache, stale up to 5 min (apps/web/lib/auth/server.ts:79-82). requireAdmin() (apps/web/lib/api-auth.ts:17-30) already works around this by re-querying DB — this route doesn't.
  • Exploit: Admin demotes abusive moderator/admin to user. Demoted user's existing session still passes stale-role check for up to 5 min → can keep deleting other users' comments via DELETE /api/v1/comments/[id].
  • Fix: Reuse fresh-DB-role pattern from requireAdmin() (or shared requireRole() helper) instead of trusting session.user.role directly.

Finding 2: Tier entitlement not re-verified against DB — LOW/MEDIUM

  • Files: apps/web/lib/auth/server.ts:79-82 (cookieCache) + all quota call sites (api/v1/ai/generate/route.ts, ai/adapt/[id]/route.ts, ai/meal-plan/generate/route.ts, ai/batch-cook/generate/route.ts, apps/web/lib/tiers.ts:31)
  • Confidence: 8
  • Description: Same cookieCache staleness as Finding 1 applies to session.user.tier, but checkAndIncrementTierLimit never re-checks DB tier.
  • Exploit: User downgraded pro→free (cancellation/chargeback). Stale session keeps pro-tier quota (AI calls, recipe count, storage) applying for up to 5 min. Bounded quota-bypass, not data breach.
  • Fix: Fetch users.tier fresh in checkAndIncrementTierLimit, mirroring requireAdmin.

No other findings ≥8 — recipes/comments/ratings/meal-plans/shopping-lists/collections/admin routes/api-keys/webhooks/Better Auth config/proxy.ts/public share pages all reviewed clean.


Round 2 — Injection & Input Validation

Finding 1: SSRF via unfollowed redirect in AI URL-import — HIGH

  • File: apps/web/lib/ai/features/import-url.ts:22-28
  • Confidence: 9
  • Description: POST /api/v1/ai/import-url validates host via validateWebhookUrl() (blocks private/loopback/metadata ranges) but then fetch(url, {...}) uses default redirect: "follow" — no restriction on where a 3xx response redirects to. apps/web/lib/webhooks.ts:39-49 already hardens this with redirect: "manual" + comment explaining why; import-url.ts never got the same fix.
  • Exploit: Attacker hosts public page returning 302 Location: http://169.254.169.254/latest/meta-data/... (or internal service). Initial host passes validation; fetch follows redirect straight to internal/metadata address, response fed into AI prompt.
  • Fix: Set redirect: "manual" in import-url.ts, treat 3xx as failure (mirror webhooks.ts).

Finding 2: DNS-rebinding TOCTOU in validateWebhookUrl — MEDIUM

  • Files: apps/web/lib/validate-webhook-url.ts:96-123, consumed by import-url.ts:22-25
  • Confidence: 8
  • Description: Validation does its own dns.lookup; the real fetch() re-resolves independently moments later. Attacker controlling authoritative DNS (low TTL) can return public IP for validation, private IP for fetch. webhooks.ts has a comment accepting this as residual risk; import-url.ts has no such mitigation and also lacks redirect protection (compounds with Finding 1).
  • Fix: Resolve host once, validate resolved IP, connect directly to that IP (pin via custom dns.lookup/agent) instead of re-resolving in fetch. Apply to both webhooks.ts and import-url.ts.

Finding 3: IDOR into S3 deleteObject via unvalidated photo storage keys — HIGH

  • Files: apps/web/app/api/v1/recipes/route.ts:48-51, apps/web/app/api/v1/recipes/[id]/route.ts:45-48,211-231, sink apps/web/lib/storage.ts:40-42
  • Confidence: 8
  • Description: photos[].key validated only as non-empty string — no check that key was actually issued to this user/recipe by /api/v1/upload/presign (no prefix check against recipes/{recipeId}/photos/ + session.user.id). Storage keys of public recipes are visible in rendered <Image> src on apps/web/app/r/[id]/page.tsx:52-58. On PUT, any photo key present in old list but absent from new list gets passed to deleteObject() with no ownership check.
  • Exploit: Attacker views victim's public recipe, copies photo storage key from image URL. Calls PUT /api/v1/recipes/{ownRecipeId} with photos: [{key: <victim's key>}] (accepted, no ownership check), then PUT again without that key → diff logic treats it as "removed" → deleteObject permanently deletes victim's S3 object. No privilege required.
  • Fix: Track storage keys server-side as belonging to a specific recipe/user (row created at presign time, or HMAC-signed key). Validate every submitted photo key was issued for this recipe/user before accepting into recipePhotos or before deletion eligibility. At minimum prefix-match against recipes/{recipeId}/ + session.user.id, and never deleteObject a key not already present in that recipe's existing photo set.

No other findings ≥8 — all sql\...`` usages parameterized correctly, no eval/exec usage, no raw filesystem path construction from user filenames elsewhere, Zod schemas otherwise bound ranges/enums/lengths properly.


Round 3 — Crypto, Secrets Management & Data Exposure

No findings ≥8. Reviewed: lib/auth/server.ts, lib/encrypt.ts, lib/gravatar.ts, lib/invites.ts, lib/api-auth.ts, shopping-list public-share feature (app/s/[id]/page.tsx, api/v1/shopping-lists/**), profile-picture upload, users/me/export, api-keys/ai-keys, webhooks, Stripe/cron signature verification, broad sweep of routes joining/returning users rows.

Notable good patterns confirmed:

  • Share-link tokens use crypto.randomUUID() (122 bits), scoped queries never leak owner PII.
  • User-row queries always project to narrow DTO before serializing (password/email never leak).
  • All security-sensitive tokens use crypto.randomBytes/randomUUID; Math.random() only in non-security contexts.
  • lib/encrypt.ts: AES-256-GCM + HKDF-SHA256, random IV per encryption, auth-tag verified.
  • Webhook/cron secret checks use crypto.timingSafeEqual with length pre-check.
  • No TLS-verification bypasses, no hardcoded secrets.
  • requireAdmin() re-queries DB role rather than trusting session cache (see Round 1 Finding 1 — inconsistently applied elsewhere).

Summary

# Severity Category Location Confidence
R2-F1 HIGH SSRF (redirect) lib/ai/features/import-url.ts:22-28 9
R2-F3 HIGH IDOR / arbitrary delete api/v1/recipes/[id]/route.ts:211-231 8
R1-F1 MEDIUM Stale-role authz bypass api/v1/comments/[id]/route.ts:33 8
R2-F2 MEDIUM SSRF (DNS rebinding) lib/validate-webhook-url.ts:96-123 8
R1-F2 LOW/MEDIUM Stale-tier quota bypass lib/tiers.ts:31 + call sites 8

Priority fix order: R2-F1 (SSRF redirect) and R2-F3 (IDOR delete) first — both HIGH, both concrete exploit paths with no special privilege needed. Then R1-F1 (moderation bypass), R2-F2 (rebinding, compounds with F1), R1-F2 (quota bypass, low impact).

Status: all 5 fixed (safeFetch w/ IP-pinning + manual-redirect in lib/validate-webhook-url.ts; isOwnedRecipePhotoKey in lib/storage.ts enforced on recipe create/update; fresh-DB role check in comments/[id]/route.ts; fresh-DB tier check in lib/tiers.ts).


Round 4 — Verification + fresh sweep

Re-verified all 5 fixes above: all PASS, no gaps, no regressions (full pnpm typecheck + pnpm test clean, same 2 pre-existing unrelated test failures as baseline main).

Finding: IDOR on rating/review photoKey — same class as R2-F3, different endpoint — HIGH (fixed)

  • File: apps/web/app/api/v1/recipes/[id]/rate/route.ts:10,41,54
  • Confidence: 9
  • Description: POST /api/v1/recipes/[id]/rate accepted a client-supplied photoKey with no ownership check — the R2-F3 fix only covered recipePhotos (folder photos/), not review photos (folder reviews/), which use a different prefix format and weren't validated at all.
  • Exploit: Attacker reads a victim's review-photo storage key off any public recipe/review response, submits it as photoKey when rating an unrelated recipe. When that recipe's author later deletes their recipe, recipes/[id]/route.ts DELETE collects all ratings.photoKey for the recipe and calls deleteObject() unconditionally — destroying the victim's photo in a completely different recipe.
  • Fix applied: added isOwnedReviewPhotoKey(key, recipeId, userId) to lib/storage.ts (checks recipes/{recipeId}/reviews/{userId}- prefix), enforced in rate/route.ts before insert/update — rejects with 400 if the submitted key wasn't issued to this user for this recipe's reviews.

No other new HIGH/MEDIUM findings survived this round. Checked clean: admin role checks, internal cron auth, public shopping-list share endpoints, all raw sql usage, other deleteObject/presign call sites.