fix(mcp): pin public IP-literal server URLs to block SSRF redirect bypass (#5244)

This commit is contained in:
Waleed
2026-06-27 12:57:21 -07:00
committed by GitHub
parent b2d43c1085
commit 9b66b40aab
2 changed files with 31 additions and 6 deletions
+19
View File
@@ -375,6 +375,20 @@ describe('validateMcpServerSsrf', () => {
)
})
it('returns the literal IP for a public IPv4 literal so the caller pins it', async () => {
await expect(validateMcpServerSsrf('http://93.184.216.34:8080/mcp')).resolves.toBe(
'93.184.216.34'
)
expect(mockDnsLookup).not.toHaveBeenCalled()
})
it('returns the literal IP for a public IPv6 literal (brackets stripped)', async () => {
await expect(
validateMcpServerSsrf('http://[2606:2800:220:1:248:1893:25c8:1946]/mcp')
).resolves.toBe('2606:2800:220:1:248:1893:25c8:1946')
expect(mockDnsLookup).not.toHaveBeenCalled()
})
it('throws McpSsrfError for cloud metadata IP literal', async () => {
await expect(validateMcpServerSsrf('http://169.254.169.254/latest/meta-data/')).rejects.toThrow(
McpSsrfError
@@ -447,6 +461,11 @@ describe('validateMcpServerSsrf', () => {
await expect(validateMcpServerSsrf('https://example.com/mcp')).resolves.toBe('93.184.216.34')
})
it('pins public IP literals on hosted so redirects cannot escape', async () => {
await expect(validateMcpServerSsrf('http://93.184.216.34/mcp')).resolves.toBe('93.184.216.34')
expect(mockDnsLookup).not.toHaveBeenCalled()
})
it('skips loopback check on hosted when allowlist is configured', async () => {
mockGetAllowedMcpDomainsFromEnv.mockReturnValue(['localhost'])
await expect(validateMcpServerSsrf('http://localhost:3000/mcp')).resolves.toBeNull()
+12 -6
View File
@@ -139,12 +139,14 @@ function isLocalhostHostname(hostname: string): boolean {
* URLs with env var references in the hostname are skipped — they will be
* validated after resolution at execution time.
*
* Returns the resolved IP address when DNS resolution was performed (so the
* caller can pin subsequent connections to that IP and prevent DNS-rebinding
* TOCTOU attacks). Returns null in cases where pinning is unnecessary or
* Returns the IP address to pin subsequent connections to (the resolved IP for
* hostnames, or the literal itself for public IP-literal URLs) so the caller can
* prevent DNS-rebinding TOCTOU attacks and stop redirects from escaping to
* internal hosts. Pinning matters for IP literals too: without it the transport
* uses the default fetch, which follows an attacker-controlled 3xx redirect to a
* private/metadata address. Returns null only when pinning is unnecessary or
* impossible: no URL, allowlist-only mode, env-var hostnames (validated later),
* IP literals (no DNS to rebind), and localhost on self-hosted (no rebinding
* risk against a fixed loopback).
* and localhost on self-hosted (no rebinding risk against a fixed loopback).
*
* @throws McpSsrfError if the URL resolves to a blocked IP address
*/
@@ -174,7 +176,11 @@ export async function validateMcpServerSsrf(url: string | undefined): Promise<st
if (isPrivateOrReservedIP(cleanHostname)) {
throw new McpSsrfError('MCP server URL cannot point to a private or reserved IP address')
}
return null
// Public IP literal: pin to this exact address so the caller's pinned fetch
// (createPinnedFetch) keeps every redirect hop on it. Returning null here
// would fall back to the default fetch, which follows a 3xx redirect to a
// private/metadata host and escapes SSRF controls.
return cleanHostname
}
let address: string