fix(security): harden OPDS proxy SSRF, storage key validation, Stripe check (#4638)

Server-side hardening for three reported web advisories: - OPDS proxy (/api/opds/proxy): add http(s) scheme allowlist, internal/loopback/ link-local host blocklist, and manual per-hop redirect re-validation so a public URL can't redirect into an internal address. Move isBlockedHost into the shared src/utils/network.ts as the canonical blocklist and reimplement isLanAddress to delegate to it (also tightens the /api/kosync LAN check); fetch-url.ts re-exports it. (GHSA-c7mm-g2j2-98cx, GHSA-5g3f-mq2c-j65v) - Storage upload (/api/storage/upload): validate the client-supplied fileName with a new isSafeObjectKeyName helper before building the object key, so a name can't escape the caller's own prefix. (GHSA-mfmj-2frf-vhgw) - Stripe (/api/stripe/check): bind the entitlement to the session owner — reject a Checkout Session whose metadata.userId differs from the authenticated caller. (GHSA-pv88-3727-j7v8) Unit tests added for each path; full suite + lint green. The Tauri-native advisory (GHSA-55vr-pvq5-6fmg) is handled in a separate change. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-18 13:45:29 +08:00
parent 2f810a70b3
commit 495783d045
11 changed files with 543 additions and 117 deletions
@@ -0,0 +1,100 @@
+import { describe, it, expect, vi, beforeEach } from 'vitest';
+import type { NextApiRequest, NextApiResponse } from 'next';
+
+// GHSA-mfmj-2frf-vhgw: a cross-tenant write via `fileName` path traversal.
+// The handler must reject a traversing `fileName` before it ever asks the
+// storage backend for a presigned PUT URL.
+
+const validateUserAndTokenMock = vi.fn();
+const getUploadSignedUrlMock = vi.fn();
+const getDownloadSignedUrlMock = vi.fn();
+const createSupabaseAdminClientMock = vi.fn();
+
+vi.mock('@/utils/cors', () => ({
+  corsAllMethods: {},
+  runMiddleware: vi.fn().mockResolvedValue(undefined),
+}));
+vi.mock('@/utils/access', () => ({
+  validateUserAndToken: (...a: unknown[]) => validateUserAndTokenMock(...a),
+  getStoragePlanData: vi.fn().mockReturnValue({ usage: 0, quota: 10 ** 12 }),
+  STORAGE_QUOTA_GRACE_BYTES: 0,
+}));
+vi.mock('@/utils/object', async (orig) => {
+  const actual = await orig<typeof import('@/utils/object')>();
+  return {
+    ...actual,
+    getUploadSignedUrl: (...a: unknown[]) => getUploadSignedUrlMock(...a),
+    getDownloadSignedUrl: (...a: unknown[]) => getDownloadSignedUrlMock(...a),
+  };
+});
+vi.mock('@/utils/supabase', () => ({
+  createSupabaseAdminClient: (...a: unknown[]) => createSupabaseAdminClientMock(...a),
+}));
+
+import handler from '@/pages/api/storage/upload';
+
+const makeReqRes = (body: Record<string, unknown>) => {
+  const req = {
+    method: 'POST',
+    headers: { authorization: 'Bearer attacker' },
+    body,
+  } as unknown as NextApiRequest;
+  const res = {
+    status: vi.fn().mockReturnThis(),
+    json: vi.fn().mockReturnThis(),
+  } as unknown as NextApiResponse;
+  return { req, res };
+};
+
+beforeEach(() => {
+  validateUserAndTokenMock.mockReset().mockResolvedValue({
+    user: { id: 'attacker-id' },
+    token: 'tok',
+  });
+  getUploadSignedUrlMock.mockReset().mockResolvedValue('https://r2/upload');
+  getDownloadSignedUrlMock.mockReset().mockResolvedValue('https://r2/download');
+
+  // Minimal chainable Supabase stub: the no-existing-record lookup then the
+  // insert, both ending in `.single()`.
+  const single = vi
+    .fn()
+    .mockResolvedValueOnce({ data: null, error: { code: 'PGRST116' } })
+    .mockResolvedValueOnce({ data: { file_size: 12345 }, error: null });
+  const builder: Record<string, unknown> = {};
+  for (const m of ['select', 'eq', 'limit', 'insert']) builder[m] = () => builder;
+  builder['single'] = single;
+  createSupabaseAdminClientMock.mockReset().mockReturnValue({ from: () => builder });
+});
+
+describe('POST /api/storage/upload — fileName traversal guard', () => {
+  it('rejects a traversing fileName with 400 and never presigns', async () => {
+    const { req, res } = makeReqRes({
+      fileName: '../victim-id/Readest/Book/hash/book.epub',
+      fileSize: 12345,
+    });
+    await handler(req, res);
+    expect(res.status).toHaveBeenCalledWith(400);
+    expect(getUploadSignedUrlMock).not.toHaveBeenCalled();
+  });
+
+  it('rejects a traversing fileName on the temp image branch too', async () => {
+    const { req, res } = makeReqRes({
+      fileName: '../../other/evil.png',
+      fileSize: 999,
+      temp: true,
+    });
+    await handler(req, res);
+    expect(res.status).toHaveBeenCalledWith(400);
+    expect(getUploadSignedUrlMock).not.toHaveBeenCalled();
+  });
+
+  it('allows a normal book key through to presigning', async () => {
+    const { req, res } = makeReqRes({
+      fileName: 'Readest/Books/hash.epub',
+      fileSize: 12345,
+    });
+    await handler(req, res);
+    expect(getUploadSignedUrlMock).toHaveBeenCalled();
+    expect(res.status).toHaveBeenCalledWith(200);
+  });
+});
@@ -0,0 +1,79 @@
+import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
+import { NextRequest } from 'next/server';
+import { GET } from '@/app/api/opds/proxy/route';
+
+// SSRF hardening for the OPDS proxy (GHSA-c7mm-g2j2-98cx / GHSA-5g3f-mq2c-j65v).
+// The proxy must refuse internal/loopback/link-local targets, non-http(s)
+// schemes, and redirects that hop to an internal address — without ever
+// reflecting the upstream body.
+
+const proxyReq = (target: string) =>
+  new NextRequest(
+    `https://web.readest.com/api/opds/proxy?url=${encodeURIComponent(target)}&stream=false`,
+  );
+
+let fetchSpy: ReturnType<typeof vi.fn>;
+
+beforeEach(() => {
+  fetchSpy = vi.fn();
+  vi.stubGlobal('fetch', fetchSpy);
+});
+
+afterEach(() => {
+  vi.unstubAllGlobals();
+  vi.restoreAllMocks();
+});
+
+describe('OPDS proxy SSRF guard', () => {
+  it('blocks the AWS metadata endpoint without fetching', async () => {
+    const res = await GET(proxyReq('http://169.254.169.254/latest/meta-data/iam/'));
+    expect(res.status).toBe(400);
+    expect(fetchSpy).not.toHaveBeenCalled();
+  });
+
+  it('blocks loopback service ports without fetching', async () => {
+    const res = await GET(proxyReq('http://127.0.0.1:6379/'));
+    expect(res.status).toBe(400);
+    expect(fetchSpy).not.toHaveBeenCalled();
+  });
+
+  it('blocks private RFC1918 ranges without fetching', async () => {
+    const res = await GET(proxyReq('http://10.0.0.10:8080/admin'));
+    expect(res.status).toBe(400);
+    expect(fetchSpy).not.toHaveBeenCalled();
+  });
+
+  it('blocks non-http(s) schemes without fetching', async () => {
+    const res = await GET(proxyReq('file:///etc/passwd'));
+    expect(res.status).toBe(400);
+    expect(fetchSpy).not.toHaveBeenCalled();
+  });
+
+  it('blocks a public URL that redirects to an internal address', async () => {
+    // First (and only) upstream hop returns a 302 pointing at the metadata IP.
+    fetchSpy.mockResolvedValueOnce(
+      new Response(null, {
+        status: 302,
+        headers: { location: 'http://169.254.169.254/latest/meta-data/' },
+      }),
+    );
+    const res = await GET(proxyReq('https://feeds.example.com/redirect'));
+    expect(res.status).toBe(400);
+    // The internal hop must never be fetched.
+    expect(fetchSpy).toHaveBeenCalledTimes(1);
+  });
+
+  it('proxies a legitimate public feed', async () => {
+    fetchSpy.mockResolvedValueOnce(
+      new Response('<feed/>', {
+        status: 200,
+        headers: { 'Content-Type': 'application/atom+xml' },
+      }),
+    );
+    const res = await GET(proxyReq('https://feeds.example.com/catalog.atom'));
+    expect(res.status).toBe(200);
+    expect(fetchSpy).toHaveBeenCalledTimes(1);
+    const body = await res.text();
+    expect(body).toContain('<feed');
+  });
+});
@@ -0,0 +1,81 @@
+import { describe, it, expect, vi, beforeEach } from 'vitest';
+
+// GHSA-pv88-3727-j7v8: `/api/stripe/check` retrieves a client-supplied
+// `sessionId` and writes the paid plan onto the *caller's* account without
+// checking the session belongs to them. A single paid session id could be
+// replayed to upgrade unlimited free accounts. The handler must reject when
+// `session.metadata.userId` does not match the authenticated caller.
+
+const retrieveMock = vi.fn();
+const validateUserAndTokenMock = vi.fn();
+const createOrUpdateSubscriptionMock = vi.fn();
+const createOrUpdatePaymentMock = vi.fn();
+
+vi.mock('@/libs/payment/stripe/server', () => ({
+  getStripe: () => ({
+    checkout: { sessions: { retrieve: (...a: unknown[]) => retrieveMock(...a) } },
+  }),
+  createOrUpdateSubscription: (...a: unknown[]) => createOrUpdateSubscriptionMock(...a),
+  createOrUpdatePayment: (...a: unknown[]) => createOrUpdatePaymentMock(...a),
+}));
+vi.mock('@/utils/access', () => ({
+  validateUserAndToken: (...a: unknown[]) => validateUserAndTokenMock(...a),
+}));
+
+import { POST } from '@/app/api/stripe/check/route';
+
+const postReq = (sessionId: string) =>
+  new Request('https://web.readest.com/api/stripe/check', {
+    method: 'POST',
+    headers: { authorization: 'Bearer caller', 'content-type': 'application/json' },
+    body: JSON.stringify({ sessionId }),
+  });
+
+beforeEach(() => {
+  validateUserAndTokenMock.mockReset().mockResolvedValue({
+    user: { id: 'caller-id' },
+    token: 'tok',
+  });
+  retrieveMock.mockReset();
+  createOrUpdateSubscriptionMock.mockReset().mockResolvedValue(undefined);
+  createOrUpdatePaymentMock.mockReset().mockResolvedValue(undefined);
+});
+
+describe('POST /api/stripe/check — session ownership', () => {
+  it('rejects a paid session owned by a different user and grants nothing', async () => {
+    retrieveMock.mockResolvedValue({
+      payment_status: 'paid',
+      subscription: 'sub_1',
+      customer: 'cus_1',
+      metadata: { userId: 'someone-else' },
+    });
+    const res = await POST(postReq('cs_live_victim'));
+    expect(res.status).toBe(403);
+    expect(createOrUpdateSubscriptionMock).not.toHaveBeenCalled();
+    expect(createOrUpdatePaymentMock).not.toHaveBeenCalled();
+  });
+
+  it('rejects a paid session with no userId metadata', async () => {
+    retrieveMock.mockResolvedValue({
+      payment_status: 'paid',
+      subscription: 'sub_1',
+      customer: 'cus_1',
+      metadata: {},
+    });
+    const res = await POST(postReq('cs_live_orphan'));
+    expect(res.status).toBe(403);
+    expect(createOrUpdateSubscriptionMock).not.toHaveBeenCalled();
+  });
+
+  it('binds a paid session that belongs to the caller', async () => {
+    retrieveMock.mockResolvedValue({
+      payment_status: 'paid',
+      subscription: 'sub_1',
+      customer: 'cus_1',
+      metadata: { userId: 'caller-id' },
+    });
+    const res = await POST(postReq('cs_live_own'));
+    expect(res.status).toBe(200);
+    expect(createOrUpdateSubscriptionMock).toHaveBeenCalledWith('caller-id', 'cus_1', 'sub_1');
+  });
+});
@@ -0,0 +1,57 @@
+import { describe, expect, it } from 'vitest';
+import { isBlockedHost, isLanAddress } from '@/utils/network';
+
+// Regression coverage for the secondary finding in GHSA-5g3f-mq2c-j65v:
+// the old `isLanAddress` only blocked the literal `127.0.0.1` / `0.0.0.0` and
+// missed whole ranges + internal hostname suffixes, so the unauthenticated
+// `/api/kosync` proxy could still reach e.g. `http://127.0.0.2:6379/`.
+// `isLanAddress` now delegates to the canonical `isBlockedHost`.
+describe('isLanAddress — hardened ranges (previously bypassable)', () => {
+  it('blocks the full 127.0.0.0/8 loopback range, not just 127.0.0.1', () => {
+    expect(isLanAddress('http://127.0.0.2:6379/')).toBe(true);
+    expect(isLanAddress('http://127.1.2.3/')).toBe(true);
+  });
+
+  it('blocks the full 0.0.0.0/8 range', () => {
+    expect(isLanAddress('http://0.1.2.3/')).toBe(true);
+  });
+
+  it('blocks 198.18.0.0/15 benchmarking and 224.0.0.0/4 multicast/reserved', () => {
+    expect(isLanAddress('http://198.18.0.1/')).toBe(true);
+    expect(isLanAddress('http://198.19.255.255/')).toBe(true);
+    expect(isLanAddress('http://225.0.0.1/')).toBe(true);
+    expect(isLanAddress('http://240.0.0.1/')).toBe(true);
+  });
+
+  it('blocks internal hostname suffixes and bare single-label hosts', () => {
+    expect(isLanAddress('http://metadata/')).toBe(true);
+    expect(isLanAddress('http://db.internal/')).toBe(true);
+    expect(isLanAddress('http://box.lan/')).toBe(true);
+    expect(isLanAddress('http://host.localhost/')).toBe(true);
+  });
+
+  it('blocks IPv4-mapped / hex-mapped IPv6 pointing at private space', () => {
+    expect(isLanAddress('http://[::ffff:127.0.0.1]/')).toBe(true);
+    expect(isLanAddress('http://[::ffff:7f00:1]/')).toBe(true);
+  });
+
+  it('still allows ordinary public catalogs', () => {
+    expect(isLanAddress('https://sync.koreader.rocks')).toBe(false);
+    expect(isLanAddress('https://8.8.8.8')).toBe(false);
+    expect(isLanAddress('https://opds.example.com/feed')).toBe(false);
+  });
+
+  it('treats invalid URLs / octets as non-LAN (URL parse rejects them)', () => {
+    expect(isLanAddress('http://10.256.0.1')).toBe(false);
+    expect(isLanAddress('not-a-url')).toBe(false);
+    expect(isLanAddress('')).toBe(false);
+  });
+});
+
+describe('isBlockedHost is exported from the canonical network helper', () => {
+  it('matches the SSRF blocklist used by the proxies', () => {
+    expect(isBlockedHost('169.254.169.254')).toBe(true);
+    expect(isBlockedHost('127.0.0.2')).toBe(true);
+    expect(isBlockedHost('example.com')).toBe(false);
+  });
+});
@@ -0,0 +1,46 @@
+import { describe, it, expect } from 'vitest';
+import { isSafeObjectKeyName } from '@/utils/object';
+
+// GHSA-mfmj-2frf-vhgw: the storage object key is built as `${user.id}/${fileName}`
+// from a client-controlled `fileName`. The R2 signer interpolates it into
+// `new Request(url)`, whose URL parser collapses `../` before signing — so a
+// crafted name escapes the caller's `${user.id}/` prefix into another tenant's
+// namespace. fileName legitimately contains '/' (Readest/Books/..., Replicas),
+// so we reject traversal/absolute/backslash forms rather than separators.
+describe('isSafeObjectKeyName', () => {
+  it('accepts the legitimate book / replica / cover key shapes', () => {
+    expect(isSafeObjectKeyName('Readest/Books/abc123.epub')).toBe(true);
+    expect(isSafeObjectKeyName('Readest/Replicas/dict/id-1/data.bin')).toBe(true);
+    expect(isSafeObjectKeyName('cover.png')).toBe(true);
+    expect(isSafeObjectKeyName('My Book (2024).epub')).toBe(true);
+    expect(isSafeObjectKeyName('A&B.epub')).toBe(true);
+  });
+
+  it('rejects parent-directory traversal segments', () => {
+    expect(isSafeObjectKeyName('../victim/Readest/Book/h/book.epub')).toBe(false);
+    expect(isSafeObjectKeyName('Readest/../../victim/book.epub')).toBe(false);
+    expect(isSafeObjectKeyName('..')).toBe(false);
+    expect(isSafeObjectKeyName('a/../b')).toBe(false);
+  });
+
+  it('rejects percent-encoded traversal', () => {
+    expect(isSafeObjectKeyName('%2e%2e/victim/book.epub')).toBe(false);
+    expect(isSafeObjectKeyName('a/%2e%2e/b')).toBe(false);
+  });
+
+  it('rejects absolute paths, backslashes, NUL and empty segments', () => {
+    expect(isSafeObjectKeyName('/etc/passwd')).toBe(false);
+    expect(isSafeObjectKeyName('a\\b')).toBe(false);
+    expect(isSafeObjectKeyName('a\0b')).toBe(false);
+    expect(isSafeObjectKeyName('a//b')).toBe(false);
+    expect(isSafeObjectKeyName('a/')).toBe(false);
+  });
+
+  it('rejects empty / non-string input', () => {
+    expect(isSafeObjectKeyName('')).toBe(false);
+    // @ts-expect-error runtime guard for untrusted req.body values
+    expect(isSafeObjectKeyName(undefined)).toBe(false);
+    // @ts-expect-error runtime guard for untrusted req.body values
+    expect(isSafeObjectKeyName(123)).toBe(false);
+  });
+});
@@ -1,6 +1,42 @@
 import { READEST_OPDS_USER_AGENT } from '@/services/constants';
 import { NextRequest, NextResponse } from 'next/server';
 import { deserializeOPDSCustomHeaders } from '@/app/opds/utils/customHeaders';
+import { isBlockedHost } from '@/utils/network';
+
+// Cap redirect hops so the SSRF host check below can re-run on every one.
+const MAX_REDIRECTS = 5;
+
+/**
+ * Fetch the target while running the SSRF host check on every redirect hop.
+ * `fetch`'s default `redirect: 'follow'` would let a public URL 302 to an
+ * internal address (e.g. the cloud metadata endpoint) after our initial host
+ * check passed, so we follow redirects manually and re-validate each hop.
+ * Throws `SsrfBlockedError` when a hop targets a blocked host or scheme.
+ */
+class SsrfBlockedError extends Error {}
+
+async function fetchFollowingRedirects(
+  startUrl: string,
+  init: { method: 'GET' | 'HEAD'; headers: Headers; signal: AbortSignal },
+): Promise<Response> {
+  let currentUrl = startUrl;
+  for (let hop = 0; hop <= MAX_REDIRECTS; hop++) {
+    const parsed = new URL(currentUrl);
+    if (parsed.protocol !== 'http:' && parsed.protocol !== 'https:') {
+      throw new SsrfBlockedError('Only http(s) URLs are supported');
+    }
+    if (isBlockedHost(parsed.hostname)) {
+      throw new SsrfBlockedError('This URL is not allowed');
+    }
+    const response = await fetch(currentUrl, { ...init, redirect: 'manual' });
+    if (response.status >= 300 && response.status < 400 && response.headers.has('location')) {
+      currentUrl = new URL(response.headers.get('location')!, currentUrl).toString();
+      continue;
+    }
+    return response;
+  }
+  throw new SsrfBlockedError('Too many redirects');
+}

 async function handleRequest(request: NextRequest, method: 'GET' | 'HEAD') {
  // Cloudflare Workers incorrectly decodes %26 to & in the url parameter value,
@@ -30,12 +66,23 @@ async function handleRequest(request: NextRequest, method: 'GET' | 'HEAD') {
    );
  }

+  let parsedUrl: URL;
  try {
-    new URL(url);
+    parsedUrl = new URL(url);
  } catch {
    return NextResponse.json({ error: 'Invalid URL format' }, { status: 400 });
  }

+  // SSRF guard: this proxy fetches an arbitrary client-supplied URL server-side,
+  // so reject non-http(s) schemes and internal/loopback/link-local targets
+  // before making any request (the redirect loop re-checks every hop).
+  if (parsedUrl.protocol !== 'http:' && parsedUrl.protocol !== 'https:') {
+    return NextResponse.json({ error: 'Only http(s) URLs are supported' }, { status: 400 });
+  }
+  if (isBlockedHost(parsedUrl.hostname)) {
+    return NextResponse.json({ error: 'This URL is not allowed' }, { status: 400 });
+  }
+
  try {
    console.log(`[OPDS Proxy] ${method}: ${url}`);

@@ -54,7 +101,7 @@ async function handleRequest(request: NextRequest, method: 'GET' | 'HEAD') {
      headers.set('Authorization', auth);
    }

-    const response = await fetch(url, {
+    const response = await fetchFollowingRedirects(url, {
      method,
      headers,
      signal: controller.signal,
@@ -200,6 +247,10 @@ async function handleRequest(request: NextRequest, method: 'GET' | 'HEAD') {
  } catch (error) {
    console.error('[OPDS Proxy] Error:', error);

+    if (error instanceof SsrfBlockedError) {
+      return NextResponse.json({ error: error.message }, { status: 400 });
+    }
+
    if (error instanceof Error) {
      if (error.name === 'AbortError') {
        return NextResponse.json(
@@ -19,6 +19,15 @@ export async function POST(request: Request) {
    const stripe = getStripe();
    const session = await stripe.checkout.sessions.retrieve(sessionId);

+    // Bind the entitlement to the session's owner, not the caller. `sessionId`
+    // is client-supplied and Stripe sessions share a global id space, so without
+    // this check any authenticated user could replay one paid session id to
+    // upgrade their own (or many) accounts (GHSA-pv88-3727-j7v8). The checkout
+    // route stamps `metadata.userId`; the webhook relies on the same field.
+    if (session.metadata?.['userId'] !== user.id) {
+      return NextResponse.json({ error: 'Session does not belong to user' }, { status: 403 });
+    }
+
    const customerId = session.customer as string;
    if (session.payment_status === 'paid' && session.subscription) {
      await createOrUpdateSubscription(user.id, customerId, session.subscription as string);
@@ -1,73 +1,16 @@
 import type { NextApiRequest, NextApiResponse } from 'next';
 import { corsAllMethods, runMiddleware } from '@/utils/cors';
 import { validateUserAndToken } from '@/utils/access';
+import { isBlockedHost } from '@/utils/network';

 const FETCH_TIMEOUT_MS = 10_000;
 const MAX_HTML_BYTES = 5 * 1024 * 1024;
 const MAX_REDIRECTS = 3;

-/** Whether a dotted-decimal IPv4 address falls in a private / reserved range. */
-function isBlockedV4(a: number, b: number, c: number, _d: number): boolean {
-  if (a === 0 || a === 10 || a === 127) return true; // this-network, private, loopback
-  if (a === 169 && b === 254) return true; // link-local
-  if (a === 172 && b >= 16 && b <= 31) return true; // private
-  if (a === 192 && b === 168) return true; // private
-  if (a === 100 && b >= 64 && b <= 127) return true; // CGNAT
-  if (a === 192 && b === 0 && c === 0) return true; // IETF protocol assignments
-  if (a === 198 && (b === 18 || b === 19)) return true; // benchmarking
-  if (a >= 224) return true; // multicast + reserved
-  return false;
-}
-
-/**
- * Block obviously-internal hosts. A browser cannot fetch arbitrary cross-origin
- * pages (CORS), so the web `/send` page routes article URLs through this proxy
- * — which means the server makes the request, so an SSRF guard is mandatory.
- * The Tauri apps fetch directly and never hit this route.
- *
- * Note: this is a string check on the URL host. A hostname that DNS-resolves to
- * a private address (DNS rebinding) is a documented residual risk — the web
- * build runs on the Cloudflare Workers edge, which has no reachable internal
- * network or metadata endpoint.
- */
-export function isBlockedHost(hostname: string): boolean {
-  const h = hostname.toLowerCase().replace(/^\[|\]$/g, '');
-  if (!h) return true;
-  if (h === 'localhost' || h.endsWith('.localhost')) return true;
-  if (h.endsWith('.local') || h.endsWith('.internal') || h.endsWith('.lan')) return true;
-  // Bare single-label hostnames (e.g. `intranet`, `metadata`) — never public.
-  if (!h.includes('.') && !h.includes(':')) return true;
-
-  // IPv4 — the WHATWG URL parser already normalized decimal/hex/octal forms.
-  const v4 = h.match(/^(\d{1,3})\.(\d{1,3})\.(\d{1,3})\.(\d{1,3})$/);
-  if (v4) {
-    return isBlockedV4(Number(v4[1]), Number(v4[2]), Number(v4[3]), Number(v4[4]));
-  }
-
-  // IPv6, including IPv4-mapped / -compatible forms.
-  if (h.includes(':')) {
-    if (h === '::' || h === '::1') return true; // unspecified, loopback
-    if (/^(fc|fd)/.test(h)) return true; // unique-local
-    if (/^fe[89ab]/.test(h)) return true; // link-local
-    const mapped = h.match(/(\d{1,3})\.(\d{1,3})\.(\d{1,3})\.(\d{1,3})$/);
-    if (mapped) {
-      return isBlockedV4(
-        Number(mapped[1]),
-        Number(mapped[2]),
-        Number(mapped[3]),
-        Number(mapped[4]),
-      );
-    }
-    const hexMapped = h.match(/::ffff:([0-9a-f]{1,4}):([0-9a-f]{1,4})$/);
-    if (hexMapped) {
-      const hi = parseInt(hexMapped[1] ?? '0', 16);
-      const lo = parseInt(hexMapped[2] ?? '0', 16);
-      return isBlockedV4((hi >> 8) & 0xff, hi & 0xff, (lo >> 8) & 0xff, lo & 0xff);
-    }
-    return false;
-  }
-  return false;
-}
+// `isBlockedHost` now lives in the shared network helper so every URL-fetching
+// route (this `/send` proxy, `/api/opds/proxy`, `/api/kosync`) uses one
+// canonical SSRF blocklist. Re-exported here for existing importers/tests.
+export { isBlockedHost };

 /** GET ?url=... — fetch a remote page's HTML for client-side article extraction. */
 export default async function handler(req: NextApiRequest, res: NextApiResponse) {
@@ -6,7 +6,7 @@ import {
  validateUserAndToken,
  STORAGE_QUOTA_GRACE_BYTES,
 } from '@/utils/access';
-import { getDownloadSignedUrl, getUploadSignedUrl } from '@/utils/object';
+import { getDownloadSignedUrl, getUploadSignedUrl, isSafeObjectKeyName } from '@/utils/object';
 import { READEST_PUBLIC_STORAGE_BASE_URL } from '@/services/constants';

 export default async function handler(req: NextApiRequest, res: NextApiResponse) {
@@ -22,6 +22,15 @@ export default async function handler(req: NextApiRequest, res: NextApiResponse)
  }

  const { fileName, fileSize, bookHash, replicaKind, replicaId, temp = false } = req.body;
+
+  // Reject object-key path traversal before building any key. `fileName` is
+  // fully client-controlled and is interpolated into `${user.id}/${fileName}`;
+  // without this an attacker escapes their own prefix into another user's
+  // namespace (GHSA-mfmj-2frf-vhgw).
+  if (!isSafeObjectKeyName(fileName)) {
+    return res.status(400).json({ error: 'Invalid fileName' });
+  }
+
  if (temp) {
    try {
      const datetime = new Date();
@@ -20,62 +20,81 @@ export const isMetered = (): boolean => {
  return connection.type === 'cellular' || connection.saveData === true;
 };

-export const isLanAddress = (url: string) => {
-  try {
-    const urlObj = new URL(url);
-    const hostname = urlObj.hostname;
+/** Whether a dotted-decimal IPv4 address falls in a private / reserved range. */
+function isBlockedV4(a: number, b: number, c: number, _d: number): boolean {
+  if (a === 0 || a === 10 || a === 127) return true; // this-network, private, loopback
+  if (a === 169 && b === 254) return true; // link-local
+  if (a === 172 && b >= 16 && b <= 31) return true; // private
+  if (a === 192 && b === 168) return true; // private
+  if (a === 100 && b >= 64 && b <= 127) return true; // CGNAT / Tailscale
+  if (a === 192 && b === 0 && c === 0) return true; // IETF protocol assignments
+  if (a === 198 && (b === 18 || b === 19)) return true; // benchmarking
+  if (a >= 224) return true; // multicast + reserved
+  return false;
+}

-    if (hostname === 'localhost' || hostname === '127.0.0.1' || hostname === '0.0.0.0') {
-      return true;
+/**
+ * Canonical SSRF host blocklist, shared by every server route that fetches a
+ * client-supplied URL (`/api/opds/proxy`, `/api/kosync`, `/api/send/fetch-url`).
+ * Returns true for hosts that must never be reached: loopback, private,
+ * link-local, CGNAT, benchmarking, multicast, internal hostname suffixes, and
+ * bare single-label names.
+ *
+ * Input is a hostname as the WHATWG URL parser serializes it (decimal/hex/octal
+ * IPv4 already normalized to dotted-quad). This is a string check — a hostname
+ * that DNS-resolves to a private address (DNS rebinding) is a documented
+ * residual risk; the hosted web build runs on the Cloudflare Workers edge,
+ * which has no reachable internal network or metadata endpoint.
+ */
+export function isBlockedHost(hostname: string): boolean {
+  const h = hostname.toLowerCase().replace(/^\[|\]$/g, '');
+  if (!h) return true;
+  if (h === 'localhost' || h.endsWith('.localhost')) return true;
+  if (h.endsWith('.local') || h.endsWith('.internal') || h.endsWith('.lan')) return true;
+  // Bare single-label hostnames (e.g. `intranet`, `metadata`) — never public.
+  if (!h.includes('.') && !h.includes(':')) return true;
+
+  // IPv4 — the WHATWG URL parser already normalized decimal/hex/octal forms.
+  const v4 = h.match(/^(\d{1,3})\.(\d{1,3})\.(\d{1,3})\.(\d{1,3})$/);
+  if (v4) {
+    return isBlockedV4(Number(v4[1]), Number(v4[2]), Number(v4[3]), Number(v4[4]));
+  }
+
+  // IPv6, including IPv4-mapped / -compatible forms.
+  if (h.includes(':')) {
+    if (h === '::' || h === '::1') return true; // unspecified, loopback
+    if (/^(fc|fd)/.test(h)) return true; // unique-local
+    if (/^fe[89ab]/.test(h)) return true; // link-local
+    const mapped = h.match(/(\d{1,3})\.(\d{1,3})\.(\d{1,3})\.(\d{1,3})$/);
+    if (mapped) {
+      return isBlockedV4(
+        Number(mapped[1]),
+        Number(mapped[2]),
+        Number(mapped[3]),
+        Number(mapped[4]),
+      );
    }
-
-    // Check for IPv4 private ranges
-    const ipv4Regex = /^(\d{1,3})\.(\d{1,3})\.(\d{1,3})\.(\d{1,3})$/;
-    const match = hostname.match(ipv4Regex);
-
-    if (match) {
-      const [, a, b, c, d] = match.map(Number);
-      if (a === undefined || b === undefined || c === undefined || d === undefined) {
-        return false;
-      }
-
-      // Validate IP format
-      if (a > 255 || b > 255 || c > 255 || d > 255) {
-        return false;
-      }
-
-      // Check private IP ranges:
-      // 10.0.0.0/8 (10.0.0.0 to 10.255.255.255)
-      if (a === 10) return true;
-
-      // 172.16.0.0/12 (172.16.0.0 to 172.31.255.255)
-      if (a === 172 && b >= 16 && b <= 31) return true;
-
-      // 192.168.0.0/16 (192.168.0.0 to 192.168.255.255)
-      if (a === 192 && b === 168) return true;
-
-      // 169.254.0.0/16 (link-local addresses)
-      if (a === 169 && b === 254) return true;
-
-      // Tailscale IPv4 range: 100.64.0.0/10 (100.64.0.0 to 100.127.255.255)
-      if (a === 100 && b >= 64 && b <= 127) return true;
+    const hexMapped = h.match(/::ffff:([0-9a-f]{1,4}):([0-9a-f]{1,4})$/);
+    if (hexMapped) {
+      const hi = parseInt(hexMapped[1] ?? '0', 16);
+      const lo = parseInt(hexMapped[2] ?? '0', 16);
+      return isBlockedV4((hi >> 8) & 0xff, hi & 0xff, (lo >> 8) & 0xff, lo & 0xff);
    }
-
-    // Check for IPv6 private addresses
-    // URL.hostname wraps IPv6 in brackets, e.g. '[::1]' — strip them
-    const ipv6 = hostname.startsWith('[') ? hostname.slice(1, -1) : hostname;
-    if (ipv6.includes(':')) {
-      if (
-        ipv6 === '::1' ||
-        ipv6.startsWith('fe80:') ||
-        ipv6.startsWith('fc00:') ||
-        ipv6.startsWith('fd00:')
-      ) {
-        return true;
-      }
-    }
-
    return false;
+  }
+  return false;
+}
+
+/**
+ * Whether a URL points at a LAN / internal address. Delegates to the canonical
+ * {@link isBlockedHost} so the proxy SSRF guard and the LAN-detection callers
+ * (KOSync direct-vs-proxy routing, OPDS catalog warnings) stay in sync.
+ * Returns false for unparseable URLs (matching the previous best-effort
+ * behavior — an invalid host can't be reached anyway).
+ */
+export const isLanAddress = (url: string): boolean => {
+  try {
+    return isBlockedHost(new URL(url).hostname);
  } catch {
    return false;
  }
@@ -2,6 +2,38 @@ import { s3Storage } from './s3';
 import { r2Storage } from './r2';
 import { getStorageType } from './storage';

+/**
+ * Whether a client-supplied `fileName` is safe to interpolate into a storage
+ * object key (`${userId}/${fileName}`). The R2 signer builds the key into
+ * `new Request(url)`, whose WHATWG URL parser collapses `../` segments before
+ * the request is signed — so an unsanitized name can escape the caller's
+ * `${userId}/` prefix and write into another tenant's namespace
+ * (GHSA-mfmj-2frf-vhgw).
+ *
+ * Legitimate names DO contain `/` (e.g. `Readest/Books/<hash>.epub`,
+ * `Readest/Replicas/<kind>/<id>/<file>`), so we reject traversal rather than
+ * separators: no `.`/`..`/empty path segments, no leading slash (absolute), no
+ * backslash or NUL, checked on both the raw and percent-decoded forms.
+ */
+export const isSafeObjectKeyName = (fileName: string): boolean => {
+  if (typeof fileName !== 'string' || fileName.length === 0) return false;
+
+  const forms = [fileName];
+  try {
+    const decoded = decodeURIComponent(fileName);
+    if (decoded !== fileName) forms.push(decoded);
+  } catch {
+    return false; // malformed percent-encoding
+  }
+
+  for (const form of forms) {
+    if (form.includes('\\') || form.includes('\0')) return false;
+    if (form.startsWith('/')) return false;
+    if (form.split('/').some((seg) => seg === '' || seg === '.' || seg === '..')) return false;
+  }
+  return true;
+};
+
 export const getDownloadSignedUrl = async (
  fileKey: string,
  expiresIn: number,