From 184de9210d6ba02c9d240adb3706d223c6a6056b Mon Sep 17 00:00:00 2001 From: Huang Xin Date: Wed, 8 Apr 2026 03:21:51 +0800 Subject: [PATCH] fix(security): prevent SSRF in kosync proxy (CWE-918) (#3793) Validate serverUrl in the kosync API proxy to block requests to private/internal addresses and non-http(s) schemes. Also fix isLanAddress to detect 0.0.0.0 and bracket-wrapped IPv6 private addresses. Closes code-scanning alert #14. Co-authored-by: Claude Opus 4.6 (1M context) --- .../src/__tests__/utils/kosync-ssrf.test.ts | 45 +++++++++++++++++++ .../src/__tests__/utils/network.test.ts | 18 ++++---- apps/readest-app/src/pages/api/kosync.ts | 16 +++++++ apps/readest-app/src/utils/network.ts | 16 ++++--- 4 files changed, 79 insertions(+), 16 deletions(-) create mode 100644 apps/readest-app/src/__tests__/utils/kosync-ssrf.test.ts diff --git a/apps/readest-app/src/__tests__/utils/kosync-ssrf.test.ts b/apps/readest-app/src/__tests__/utils/kosync-ssrf.test.ts new file mode 100644 index 00000000..be4b4bbf --- /dev/null +++ b/apps/readest-app/src/__tests__/utils/kosync-ssrf.test.ts @@ -0,0 +1,45 @@ +import { describe, it, expect } from 'vitest'; +import { isLanAddress } from '@/utils/network'; + +/** + * Tests for SSRF protection in the kosync proxy. + * The proxy must reject requests to private/internal addresses. + * See: https://github.com/readest/readest/security/code-scanning/14 + */ +describe('isLanAddress – SSRF edge cases for proxy', () => { + // 0.0.0.0 routes to localhost on many systems + it('returns true for 0.0.0.0', () => { + expect(isLanAddress('http://0.0.0.0')).toBe(true); + expect(isLanAddress('http://0.0.0.0:8080')).toBe(true); + }); + + // Cloud metadata endpoint (169.254.x.x is already covered, but test the exact AWS one) + it('returns true for cloud metadata IP 169.254.169.254', () => { + expect(isLanAddress('http://169.254.169.254')).toBe(true); + expect(isLanAddress('http://169.254.169.254/latest/meta-data/')).toBe(true); + }); + + // IPv6 loopback with brackets (URL standard format) + it('returns true for bracket-wrapped IPv6 loopback [::1]', () => { + expect(isLanAddress('http://[::1]')).toBe(true); + expect(isLanAddress('http://[::1]:8080')).toBe(true); + }); + + // IPv6 link-local with brackets + it('returns true for bracket-wrapped IPv6 link-local [fe80::1]', () => { + expect(isLanAddress('http://[fe80::1]')).toBe(true); + }); + + // IPv6 unique local with brackets + it('returns true for bracket-wrapped IPv6 unique local [fc00::1] and [fd00::1]', () => { + expect(isLanAddress('http://[fc00::1]')).toBe(true); + expect(isLanAddress('http://[fd00::abc]')).toBe(true); + }); + + // Public addresses should still return false + it('returns false for public addresses', () => { + expect(isLanAddress('https://sync.koreader.rocks')).toBe(false); + expect(isLanAddress('https://8.8.8.8')).toBe(false); + expect(isLanAddress('http://[2001:db8::1]')).toBe(false); + }); +}); diff --git a/apps/readest-app/src/__tests__/utils/network.test.ts b/apps/readest-app/src/__tests__/utils/network.test.ts index 030d78a0..09ee7f29 100644 --- a/apps/readest-app/src/__tests__/utils/network.test.ts +++ b/apps/readest-app/src/__tests__/utils/network.test.ts @@ -76,15 +76,15 @@ describe('isLanAddress', () => { // IPv6 private addresses // ----------------------------------------------------------------------- describe('IPv6 private addresses', () => { - // Note: URL.hostname wraps IPv6 addresses in brackets (e.g., '[::1]'), - // so the current implementation's startsWith checks won't match bracket- - // wrapped addresses. These tests document the actual behavior. - it('returns false for bracket-wrapped IPv6 (URL standard hostname format)', () => { - // URL('http://[::1]').hostname === '[::1]' which doesn't match startsWith('::1') - expect(isLanAddress('http://[::1]')).toBe(false); - expect(isLanAddress('http://[fe80::1]')).toBe(false); - expect(isLanAddress('http://[fc00::1]')).toBe(false); - expect(isLanAddress('http://[fd00::abc]')).toBe(false); + it('returns true for bracket-wrapped IPv6 loopback', () => { + expect(isLanAddress('http://[::1]')).toBe(true); + expect(isLanAddress('http://[::1]:8080')).toBe(true); + }); + + it('returns true for bracket-wrapped IPv6 link-local and unique local', () => { + expect(isLanAddress('http://[fe80::1]')).toBe(true); + expect(isLanAddress('http://[fc00::1]')).toBe(true); + expect(isLanAddress('http://[fd00::abc]')).toBe(true); }); }); diff --git a/apps/readest-app/src/pages/api/kosync.ts b/apps/readest-app/src/pages/api/kosync.ts index 1054c486..1c8219e1 100644 --- a/apps/readest-app/src/pages/api/kosync.ts +++ b/apps/readest-app/src/pages/api/kosync.ts @@ -1,5 +1,6 @@ import type { NextApiRequest, NextApiResponse } from 'next'; import { corsAllMethods, runMiddleware } from '@/utils/cors'; +import { isLanAddress } from '@/utils/network'; import { KoSyncProxyPayload } from '@/types/kosync'; const validEndpoints = [/\/users\/create/, /\/users\/auth/, /\/syncs\/progress/]; @@ -27,6 +28,21 @@ export default async function handler(req: NextApiRequest, res: NextApiResponse) return res.status(400).json({ error: 'Invalid endpoint' }); } + try { + const parsed = new URL(serverUrl); + if (parsed.protocol !== 'http:' && parsed.protocol !== 'https:') { + return res.status(400).json({ error: 'Only http and https URLs are allowed' }); + } + } catch { + return res.status(400).json({ error: 'Invalid serverUrl' }); + } + + if (isLanAddress(serverUrl)) { + return res + .status(400) + .json({ error: 'Requests to private/internal addresses are not allowed' }); + } + const targetUrl = `${serverUrl.replace(/\/$/, '')}${endpoint}`; try { diff --git a/apps/readest-app/src/utils/network.ts b/apps/readest-app/src/utils/network.ts index 77ea8c6b..b2b82ecb 100644 --- a/apps/readest-app/src/utils/network.ts +++ b/apps/readest-app/src/utils/network.ts @@ -3,7 +3,7 @@ export const isLanAddress = (url: string) => { const urlObj = new URL(url); const hostname = urlObj.hostname; - if (hostname === 'localhost' || hostname === '127.0.0.1') { + if (hostname === 'localhost' || hostname === '127.0.0.1' || hostname === '0.0.0.0') { return true; } @@ -39,13 +39,15 @@ export const isLanAddress = (url: string) => { if (a === 100 && b >= 64 && b <= 127) return true; } - // Check for IPv6 private addresses (simplified check) - if (hostname.includes(':')) { + // 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 ( - hostname.startsWith('::1') || - hostname.startsWith('fe80:') || - hostname.startsWith('fc00:') || - hostname.startsWith('fd00:') + ipv6 === '::1' || + ipv6.startsWith('fe80:') || + ipv6.startsWith('fc00:') || + ipv6.startsWith('fd00:') ) { return true; }