Skip to content

Commit 858e634

Browse files
committed
fix(mcp): pin public IP-literal server URLs to block SSRF redirect bypass
1 parent 0420fee commit 858e634

2 files changed

Lines changed: 31 additions & 6 deletions

File tree

apps/sim/lib/mcp/domain-check.test.ts

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -375,6 +375,20 @@ describe('validateMcpServerSsrf', () => {
375375
)
376376
})
377377

378+
it('returns the literal IP for a public IPv4 literal so the caller pins it', async () => {
379+
await expect(validateMcpServerSsrf('http://93.184.216.34:8080/mcp')).resolves.toBe(
380+
'93.184.216.34'
381+
)
382+
expect(mockDnsLookup).not.toHaveBeenCalled()
383+
})
384+
385+
it('returns the literal IP for a public IPv6 literal (brackets stripped)', async () => {
386+
await expect(
387+
validateMcpServerSsrf('http://[2606:2800:220:1:248:1893:25c8:1946]/mcp')
388+
).resolves.toBe('2606:2800:220:1:248:1893:25c8:1946')
389+
expect(mockDnsLookup).not.toHaveBeenCalled()
390+
})
391+
378392
it('throws McpSsrfError for cloud metadata IP literal', async () => {
379393
await expect(validateMcpServerSsrf('http://169.254.169.254/latest/meta-data/')).rejects.toThrow(
380394
McpSsrfError
@@ -447,6 +461,11 @@ describe('validateMcpServerSsrf', () => {
447461
await expect(validateMcpServerSsrf('https://example.com/mcp')).resolves.toBe('93.184.216.34')
448462
})
449463

464+
it('pins public IP literals on hosted so redirects cannot escape', async () => {
465+
await expect(validateMcpServerSsrf('http://93.184.216.34/mcp')).resolves.toBe('93.184.216.34')
466+
expect(mockDnsLookup).not.toHaveBeenCalled()
467+
})
468+
450469
it('skips loopback check on hosted when allowlist is configured', async () => {
451470
mockGetAllowedMcpDomainsFromEnv.mockReturnValue(['localhost'])
452471
await expect(validateMcpServerSsrf('http://localhost:3000/mcp')).resolves.toBeNull()

apps/sim/lib/mcp/domain-check.ts

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -139,12 +139,14 @@ function isLocalhostHostname(hostname: string): boolean {
139139
* URLs with env var references in the hostname are skipped — they will be
140140
* validated after resolution at execution time.
141141
*
142-
* Returns the resolved IP address when DNS resolution was performed (so the
143-
* caller can pin subsequent connections to that IP and prevent DNS-rebinding
144-
* TOCTOU attacks). Returns null in cases where pinning is unnecessary or
142+
* Returns the IP address to pin subsequent connections to (the resolved IP for
143+
* hostnames, or the literal itself for public IP-literal URLs) so the caller can
144+
* prevent DNS-rebinding TOCTOU attacks and stop redirects from escaping to
145+
* internal hosts. Pinning matters for IP literals too: without it the transport
146+
* uses the default fetch, which follows an attacker-controlled 3xx redirect to a
147+
* private/metadata address. Returns null only when pinning is unnecessary or
145148
* impossible: no URL, allowlist-only mode, env-var hostnames (validated later),
146-
* IP literals (no DNS to rebind), and localhost on self-hosted (no rebinding
147-
* risk against a fixed loopback).
149+
* and localhost on self-hosted (no rebinding risk against a fixed loopback).
148150
*
149151
* @throws McpSsrfError if the URL resolves to a blocked IP address
150152
*/
@@ -174,7 +176,11 @@ export async function validateMcpServerSsrf(url: string | undefined): Promise<st
174176
if (isPrivateOrReservedIP(cleanHostname)) {
175177
throw new McpSsrfError('MCP server URL cannot point to a private or reserved IP address')
176178
}
177-
return null
179+
// Public IP literal: pin to this exact address so the caller's pinned fetch
180+
// (createPinnedFetch) keeps every redirect hop on it. Returning null here
181+
// would fall back to the default fetch, which follows a 3xx redirect to a
182+
// private/metadata host and escapes SSRF controls.
183+
return cleanHostname
178184
}
179185

180186
let address: string

0 commit comments

Comments
 (0)