🛡️ Sentinel: [HIGH] Fix SSRF via DNS pre-fetch validation in url lookup route#41
🛡️ Sentinel: [HIGH] Fix SSRF via DNS pre-fetch validation in url lookup route#41aicoder2009 wants to merge 2 commits intomainfrom
Conversation
Co-authored-by: aicoder2009 <[email protected]>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Pull request overview
Adds DNS pre-resolution checks to the URL lookup API to reduce SSRF risk by blocking requests whose hostnames resolve to private/internal IP ranges, and updates tests/documentation accordingly.
Changes:
- Add
dns.lookup()pre-fetch validation and a private-IP filter in/api/lookup/url. - Mock
dns/promisesin the URL lookup route unit tests to keep tests deterministic. - Record the SSRF learning/prevention note in
.jules/sentinel.md.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/app/api/lookup/url/route.ts | Introduces DNS-based hostname resolution and private-IP blocking before fetching user-supplied URLs. |
| src/app/api/lookup/url/route.test.ts | Adds a global mock for dns/promises so route tests don’t perform real DNS lookups. |
| .jules/sentinel.md | Documents the SSRF vulnerability pattern and prevention guidance. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| import type { SourceType } from '@/types'; | ||
|
|
||
| /** | ||
| * Checks if an IP address is a private/internal IP. | ||
| * Helps prevent Server-Side Request Forgery (SSRF). | ||
| */ | ||
| function isPrivateIP(ip: string): boolean { | ||
| const parts = ip.split('.'); | ||
| if (parts.length === 4) { | ||
| const [a, b] = parts.map(Number); | ||
| return ( | ||
| a === 10 || // 10.0.0.0/8 | ||
| (a === 172 && b >= 16 && b <= 31) || // 172.16.0.0/12 | ||
| (a === 192 && b === 168) || // 192.168.0.0/16 | ||
| a === 127 || // 127.0.0.0/8 | ||
| a === 0 || // 0.0.0.0/8 | ||
| (a === 169 && b === 254) // 169.254.0.0/16 | ||
| ); | ||
| } | ||
| // Basic IPv6 check for localhost/private | ||
| return ip === '::1' || ip.startsWith('fe80:') || ip.startsWith('fc00:') || ip.startsWith('fd00:'); | ||
| } | ||
|
|
There was a problem hiding this comment.
isPrivateIP doesn't catch IPv4-mapped IPv6 addresses (e.g. ::ffff:127.0.0.1, ::ffff:169.254.169.254). A URL like http://[::ffff:127.0.0.1]/ would bypass the private-IP check and still allow SSRF. Consider normalizing IPv6 (including ::ffff: mapped addresses) and using a more robust IP-range check (e.g. net.isIP + parsing to bytes / an IP library) before deciding it's public.
| import type { SourceType } from '@/types'; | |
| /** | |
| * Checks if an IP address is a private/internal IP. | |
| * Helps prevent Server-Side Request Forgery (SSRF). | |
| */ | |
| function isPrivateIP(ip: string): boolean { | |
| const parts = ip.split('.'); | |
| if (parts.length === 4) { | |
| const [a, b] = parts.map(Number); | |
| return ( | |
| a === 10 || // 10.0.0.0/8 | |
| (a === 172 && b >= 16 && b <= 31) || // 172.16.0.0/12 | |
| (a === 192 && b === 168) || // 192.168.0.0/16 | |
| a === 127 || // 127.0.0.0/8 | |
| a === 0 || // 0.0.0.0/8 | |
| (a === 169 && b === 254) // 169.254.0.0/16 | |
| ); | |
| } | |
| // Basic IPv6 check for localhost/private | |
| return ip === '::1' || ip.startsWith('fe80:') || ip.startsWith('fc00:') || ip.startsWith('fd00:'); | |
| } | |
| import net from 'net'; | |
| import type { SourceType } from '@/types'; | |
| /** | |
| * Checks if an IP address is a private/internal IP. | |
| * Helps prevent Server-Side Request Forgery (SSRF). | |
| */ | |
| function parseIPv4(ip: string): number[] | null { | |
| const parts = ip.split('.'); | |
| if (parts.length !== 4) { | |
| return null; | |
| } | |
| const bytes = parts.map((part) => Number(part)); | |
| if (bytes.some((byte, index) => !/^\d+$/.test(parts[index]) || byte < 0 || byte > 255)) { | |
| return null; | |
| } | |
| return bytes; | |
| } | |
| function isPrivateIPv4(bytes: number[]): boolean { | |
| const [a, b] = bytes; | |
| return ( | |
| a === 10 || // 10.0.0.0/8 | |
| (a === 172 && b >= 16 && b <= 31) || // 172.16.0.0/12 | |
| (a === 192 && b === 168) || // 192.168.0.0/16 | |
| a === 127 || // 127.0.0.0/8 | |
| a === 0 || // 0.0.0.0/8 | |
| (a === 169 && b === 254) // 169.254.0.0/16 | |
| ); | |
| } | |
| function parseIPv6(ip: string): number[] | null { | |
| let normalized = ip; | |
| if (normalized.includes('%')) { | |
| normalized = normalized.split('%')[0]; | |
| } | |
| const halves = normalized.split('::'); | |
| if (halves.length > 2) { | |
| return null; | |
| } | |
| const parseHextetGroup = (group: string): number[] | null => { | |
| if (!group) { | |
| return []; | |
| } | |
| const tokens = group.split(':'); | |
| const result: number[] = []; | |
| for (const token of tokens) { | |
| if (!token) { | |
| return null; | |
| } | |
| if (token.includes('.')) { | |
| const ipv4Bytes = parseIPv4(token); | |
| if (!ipv4Bytes) { | |
| return null; | |
| } | |
| result.push((ipv4Bytes[0] << 8) | ipv4Bytes[1], (ipv4Bytes[2] << 8) | ipv4Bytes[3]); | |
| } else { | |
| if (!/^[0-9a-fA-F]{1,4}$/.test(token)) { | |
| return null; | |
| } | |
| result.push(parseInt(token, 16)); | |
| } | |
| } | |
| return result; | |
| }; | |
| const left = parseHextetGroup(halves[0] ?? ''); | |
| const right = parseHextetGroup(halves[1] ?? ''); | |
| if (!left || !right) { | |
| return null; | |
| } | |
| let hextets: number[]; | |
| if (halves.length === 2) { | |
| const zerosToInsert = 8 - (left.length + right.length); | |
| if (zerosToInsert < 1) { | |
| return null; | |
| } | |
| hextets = [...left, ...new Array(zerosToInsert).fill(0), ...right]; | |
| } else { | |
| if (left.length !== 8) { | |
| return null; | |
| } | |
| hextets = left; | |
| } | |
| if (hextets.length !== 8) { | |
| return null; | |
| } | |
| return hextets.flatMap((hextet) => [(hextet >> 8) & 0xff, hextet & 0xff]); | |
| } | |
| function isPrivateIPv6(bytes: number[]): boolean { | |
| if (bytes.length !== 16) { | |
| return false; | |
| } | |
| const isUnspecified = bytes.every((byte) => byte === 0); | |
| if (isUnspecified) { | |
| return true; | |
| } | |
| const isLoopback = bytes.slice(0, 15).every((byte) => byte === 0) && bytes[15] === 1; | |
| if (isLoopback) { | |
| return true; | |
| } | |
| const isLinkLocal = bytes[0] === 0xfe && (bytes[1] & 0xc0) === 0x80; // fe80::/10 | |
| if (isLinkLocal) { | |
| return true; | |
| } | |
| const isUniqueLocal = (bytes[0] & 0xfe) === 0xfc; // fc00::/7 | |
| if (isUniqueLocal) { | |
| return true; | |
| } | |
| const isIPv4Mapped = | |
| bytes.slice(0, 10).every((byte) => byte === 0) && | |
| bytes[10] === 0xff && | |
| bytes[11] === 0xff; | |
| if (isIPv4Mapped) { | |
| return isPrivateIPv4(bytes.slice(12, 16)); | |
| } | |
| return false; | |
| } | |
| function isPrivateIP(ip: string): boolean { | |
| const ipVersion = net.isIP(ip); | |
| if (ipVersion === 4) { | |
| const bytes = parseIPv4(ip); | |
| return bytes ? isPrivateIPv4(bytes) : false; | |
| } | |
| if (ipVersion === 6) { | |
| const bytes = parseIPv6(ip); | |
| return bytes ? isPrivateIPv6(bytes) : false; | |
| } | |
| return false; | |
| } |
| // SSRF Prevention: Resolve the hostname and block internal/private IP networks | ||
| try { | ||
| const records = await dns.lookup(parsedUrl.hostname, { all: true }); | ||
| if (records.some((r) => isPrivateIP(r.address))) { | ||
| return NextResponse.json( | ||
| { success: false, error: 'Access to internal networks is forbidden' }, | ||
| { status: 403 } | ||
| ); | ||
| } |
There was a problem hiding this comment.
The SSRF mitigation only validates the initial hostname's DNS result. fetch() follows redirects by default, so an attacker could supply a public URL that redirects to a private/loopback/link-local address and bypass this check. To close this gap, either disable redirects (redirect: 'manual') or implement redirect-chain validation (resolving and checking each Location target before following).
| // Mock dns globally to prevent SSRF block from failing tests | ||
| vi.mock('dns/promises', () => ({ | ||
| default: { | ||
| lookup: vi.fn().mockResolvedValue([{ address: '93.184.216.34', family: 4 }]) | ||
| } | ||
| })); |
There was a problem hiding this comment.
New DNS-based SSRF blocking behavior is introduced, but the tests only mock dns.lookup to always return a public IP and don't assert the new outcomes. Please add coverage for (1) blocking a private/loopback resolution (expect 403 and no fetch), and (2) DNS resolution failures (expect the intended error/status).
🛡️ Sentinel: [HIGH] Fix SSRF in URL Lookup
🚨 Severity: HIGH
💡 Vulnerability: The
/api/lookup/url/route.tsendpoint previously fetched user-supplied URLs blindly. It failed to resolve and validate the underlying IP address, rendering it susceptible to Server-Side Request Forgery (SSRF) against internal or private network blocks.🎯 Impact: An attacker could provide a URL pointing to
localhost,127.0.0.1, AWS metadata169.254.169.254, or other private RFC 1918 IPs, causing the server backend to fetch and leak internal configuration or perform unintended actions behind the firewall.🔧 Fix: Added a pre-fetch step using
dns.lookupto resolve the parsed hostname. The request is aborted with a 403 status if the resolved IP points to private subnets.route.test.tshas been mocked appropriately.✅ Verification:
pnpm testpasses smoothly with mockeddns/promises. A manual build was verified.PR created automatically by Jules for task 7980900257698640724 started by @aicoder2009