Skip to content

🛡️ Sentinel: [HIGH] Fix SSRF vulnerability in /api/lookup/url#30

Open
aicoder2009 wants to merge 2 commits intomainfrom
sentinel/fix-ssrf-fetch-1917711986007538456
Open

🛡️ Sentinel: [HIGH] Fix SSRF vulnerability in /api/lookup/url#30
aicoder2009 wants to merge 2 commits intomainfrom
sentinel/fix-ssrf-fetch-1917711986007538456

Conversation

@aicoder2009
Copy link
Copy Markdown
Owner

🚨 Severity: HIGH
💡 Vulnerability: The /api/lookup/url endpoint uses Node.js native fetch to scrape arbitrary URLs supplied by the user without checking the resolved IP address.
🎯 Impact: Attackers could supply internal/private IP addresses or perform DNS rebinding to probe internal network services, leading to a Server-Side Request Forgery (SSRF) vulnerability.
🔧 Fix: Implemented pre-fetch DNS validation using dns.lookup to ensure the target IP address does not fall within private, loopback, or reserved IP ranges. Restricted protocols to HTTP/HTTPS.
✅ Verification: Ensure the updated test suite passes (pnpm test src/app/api/lookup/url/route.test.ts), which checks blocking logic for loopback and other private IP subnets.


PR created automatically by Jules for task 1917711986007538456 started by @aicoder2009

Co-authored-by: aicoder2009 <127642633+aicoder2009@users.noreply.github.com>
@google-labs-jules
Copy link
Copy Markdown
Contributor

👋 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 @jules. You can find this option in the Pull Request section of your global Jules UI settings. You can always switch back!

New to Jules? Learn more at jules.google/docs.


For security, I will only act on instructions from the user who triggered this task.

Copilot AI review requested due to automatic review settings April 27, 2026 13:10
@vercel
Copy link
Copy Markdown

vercel Bot commented Apr 27, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
opencitation Ready Ready Preview, Comment May 1, 2026 6:22pm

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Mitigates a reported SSRF risk in /api/lookup/url by adding pre-fetch URL protocol restrictions and DNS-based IP validation, along with new unit tests covering the blocking behavior.

Changes:

  • Added protocol allowlist (HTTP/HTTPS only) for the lookup URL.
  • Added DNS resolution + IP range checks intended to block private/loopback/reserved targets.
  • Added Vitest coverage for invalid protocol, loopback IP blocking, and DNS lookup failures; updated Sentinel documentation.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.

File Description
src/app/api/lookup/url/route.ts Adds protocol restriction and DNS/IP checks before fetching user-supplied URLs.
src/app/api/lookup/url/route.test.ts Mocks dns/promises.lookup and adds SSRF-related unit tests.
.jules/sentinel.md Documents the SSRF vulnerability pattern and mitigation tradeoffs.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 13 to +46
import type { SourceType } from '@/types';

// Utility to check if an IP is private/local
function isPrivateIP(ip: string): boolean {
// IPv4 Private/Reserved Ranges
if (
ip.startsWith('10.') ||
ip.startsWith('127.') ||
ip.startsWith('169.254.') ||
ip.startsWith('192.168.') ||
ip === '0.0.0.0' ||
ip === '255.255.255.255'
) {
return true;
}

// 172.16.0.0/12
if (ip.startsWith('172.')) {
const secondOctet = parseInt(ip.split('.')[1], 10);
if (secondOctet >= 16 && secondOctet <= 31) return true;
}

// IPv6 Private/Reserved Ranges
if (
ip === '::1' ||
ip === '::' ||
ip.toLowerCase().startsWith('fc00:') ||
ip.toLowerCase().startsWith('fd00:') ||
ip.toLowerCase().startsWith('fe80:')
) {
return true;
}

return false;
Copy link

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The IPv6 checks in isPrivateIP are incomplete for the CIDR ranges being targeted: fc00::/7 is not equivalent to startsWith('fc00:')/startsWith('fd00:') (e.g. fd12::1 would be allowed), and fe80::/10 is not equivalent to startsWith('fe80:') (e.g. fe9a:: would be allowed). This leaves bypasses for internal/ULA/link-local IPv6 addresses; consider using a proper IP parser + CIDR range checks (e.g., ipaddr.js or similar) instead of string prefixes.

Suggested change
import type { SourceType } from '@/types';
// Utility to check if an IP is private/local
function isPrivateIP(ip: string): boolean {
// IPv4 Private/Reserved Ranges
if (
ip.startsWith('10.') ||
ip.startsWith('127.') ||
ip.startsWith('169.254.') ||
ip.startsWith('192.168.') ||
ip === '0.0.0.0' ||
ip === '255.255.255.255'
) {
return true;
}
// 172.16.0.0/12
if (ip.startsWith('172.')) {
const secondOctet = parseInt(ip.split('.')[1], 10);
if (secondOctet >= 16 && secondOctet <= 31) return true;
}
// IPv6 Private/Reserved Ranges
if (
ip === '::1' ||
ip === '::' ||
ip.toLowerCase().startsWith('fc00:') ||
ip.toLowerCase().startsWith('fd00:') ||
ip.toLowerCase().startsWith('fe80:')
) {
return true;
}
return false;
import * as ipaddr from 'ipaddr.js';
import type { SourceType } from '@/types';
// Utility to check if an IP is private/local
function isPrivateIP(ip: string): boolean {
try {
const addr = ipaddr.parse(ip);
if (addr.kind() === 'ipv4') {
return (
addr.match(ipaddr.parseCIDR('10.0.0.0/8')) ||
addr.match(ipaddr.parseCIDR('127.0.0.0/8')) ||
addr.match(ipaddr.parseCIDR('169.254.0.0/16')) ||
addr.match(ipaddr.parseCIDR('172.16.0.0/12')) ||
addr.match(ipaddr.parseCIDR('192.168.0.0/16')) ||
addr.toString() === '0.0.0.0' ||
addr.toString() === '255.255.255.255'
);
}
return (
addr.toString() === '::1' ||
addr.toString() === '::' ||
addr.match(ipaddr.parseCIDR('fc00::/7')) ||
addr.match(ipaddr.parseCIDR('fe80::/10'))
);
} catch {
return false;
}

Copilot uses AI. Check for mistakes.
Comment on lines +126 to +133
const { address } = await lookup(parsedUrl.hostname);
if (isPrivateIP(address)) {
return NextResponse.json(
{ success: false, error: 'Access to internal networks is forbidden' },
{ status: 403 }
);
}
resolvedIp = address;
Copy link

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dns.lookup(parsedUrl.hostname) only returns a single address. If the hostname has multiple A/AAAA records (or changes between the check and fetch), the pre-check can pass on a public IP while the actual fetch resolves to a private/loopback/link-local IP. Consider calling lookup with { all: true } and rejecting if any resolved address is private, and/or pinning resolution at request time via a custom dispatcher/agent if you need stronger DNS-rebinding resistance.

Suggested change
const { address } = await lookup(parsedUrl.hostname);
if (isPrivateIP(address)) {
return NextResponse.json(
{ success: false, error: 'Access to internal networks is forbidden' },
{ status: 403 }
);
}
resolvedIp = address;
const addresses = await lookup(parsedUrl.hostname, { all: true });
if (addresses.length === 0) {
return NextResponse.json(
{ success: false, error: 'Could not resolve hostname' },
{ status: 400 }
);
}
if (addresses.some(({ address }) => isPrivateIP(address))) {
return NextResponse.json(
{ success: false, error: 'Access to internal networks is forbidden' },
{ status: 403 }
);
}
resolvedIp = addresses[0].address;

Copilot uses AI. Check for mistakes.
Comment on lines +115 to +122
// SSRF Protection: Only allow HTTP and HTTPS
if (parsedUrl.protocol !== 'http:' && parsedUrl.protocol !== 'https:') {
return NextResponse.json(
{ success: false, error: 'Invalid protocol' },
{ status: 400 }
);
}

Copy link

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even with the pre-fetch DNS/IP validation, fetch() will follow redirects by default. A request to a public URL that 3xx-redirects to an internal host/IP would bypass the current guard because only the original hostname is validated. Consider using redirect: 'manual' and explicitly validating (and optionally following) redirect targets with the same SSRF checks.

Copilot uses AI. Check for mistakes.
Comment on lines +124 to +134
let resolvedIp: string;
try {
const { address } = await lookup(parsedUrl.hostname);
if (isPrivateIP(address)) {
return NextResponse.json(
{ success: false, error: 'Access to internal networks is forbidden' },
{ status: 403 }
);
}
resolvedIp = address;
} catch (dnsError) {
Copy link

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

resolvedIp is assigned but never used. Either remove it, or use it for something meaningful (e.g., logging/telemetry), otherwise it becomes dead code and can mislead readers into thinking the resolved IP is being used/pinned for the subsequent request.

Copilot uses AI. Check for mistakes.
Comment on lines +62 to +93
it('should return error for private IP addresses (SSRF protection)', async () => {
// Mock lookup to return a private IP
vi.mocked(lookup).mockResolvedValueOnce({ address: '127.0.0.1', family: 4 });

const request = new NextRequest('http://localhost/api/lookup/url', {
method: 'POST',
body: JSON.stringify({ url: 'http://localhost:8080/admin' }),
});

const response = await POST(request);
const data = await response.json();

expect(response.status).toBe(403);
expect(data.success).toBe(false);
expect(data.error).toContain('Access to internal networks is forbidden');
});

it('should return error if DNS lookup fails', async () => {
vi.mocked(lookup).mockRejectedValueOnce(new Error('ENOTFOUND'));

const request = new NextRequest('http://localhost/api/lookup/url', {
method: 'POST',
body: JSON.stringify({ url: 'http://this-domain-does-not-exist-1234.com' }),
});

const response = await POST(request);
const data = await response.json();

expect(response.status).toBe(400);
expect(data.success).toBe(false);
expect(data.error).toContain('Could not resolve hostname');
});
Copy link

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new SSRF tests cover invalid protocol, loopback IPv4, and DNS failures, but they don’t exercise important bypass cases introduced by the implementation: IPv6 ULA/link-local (e.g. fdxx::/8, fe8x::/10), multi-address DNS results, and redirects to internal networks. Adding test cases for these would help ensure the protections actually hold for the intended threat model.

Copilot uses AI. Check for mistakes.
@aicoder2009 aicoder2009 closed this May 1, 2026
@aicoder2009 aicoder2009 reopened this May 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants