Skip to content

Commit 04ef846

Browse files
herdiyana256herdiyana256
committed
fix(mcp): add SSRF protection to OAuth metadata discovery
The OAuth discovery flow in oauth-utils.ts and oauth-provider.ts fetches URLs received directly from external MCP server responses without any SSRF validation, unlike web-fetch.ts which already uses isLoopbackHost() and resolveAndValidateDns() from utils/fetch.ts. A malicious MCP server can return private/internal IPs in: - WWW-Authenticate: Bearer resource_metadata="http://169.254.169.254/..." - /.well-known/oauth-protected-resource: {authorization_servers: ["http://169.254.169.254"]} - /.well-known/oauth-authorization-server: {registration_endpoint: "http://10.x.x.x/admin"} This causes gemini-cli to make GET and POST requests to cloud IMDS endpoints (AWS 169.254.169.254, GCP metadata.google.internal, Azure 169.254.169.254) or internal services on the developer's network. Fix: apply the same isLoopbackHost() + resolveAndValidateDns() guards already used in web-fetch.ts to: - OAuthUtils.fetchProtectedResourceMetadata() - OAuthUtils.fetchAuthorizationServerMetadata() - MCPOAuthProvider.registerClient()
1 parent 17a70c7 commit 04ef846

5 files changed

Lines changed: 164 additions & 44 deletions

File tree

‎packages/core/src/mcp/oauth-provider.ts‎

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@ import { OAuthUtils, ResourceMismatchError } from './oauth-utils.js';
1414
import { coreEvents } from '../utils/events.js';
1515
import { debugLogger } from '../utils/debugLogger.js';
1616
import { getConsentForOauth } from '../utils/authConsent.js';
17+
import { isLoopbackHost, resolveAndValidateDns } from '../utils/fetch.js';
18+
import { Agent, type Dispatcher } from 'undici';
1719
import {
1820
generatePKCEParams,
1921
startCallbackServer,
@@ -111,13 +113,44 @@ export class MCPOAuthProvider {
111113
scope: config.scopes?.join(' ') || '',
112114
};
113115

116+
const parsedRegUrl = new URL(registrationUrl);
117+
if (isLoopbackHost(parsedRegUrl.hostname)) {
118+
throw new Error(
119+
`Client registration blocked: loopback address not allowed: ${registrationUrl}`,
120+
);
121+
}
122+
const resolvedAddrs = await resolveAndValidateDns(registrationUrl);
123+
if (resolvedAddrs.length === 0) {
124+
throw new Error(
125+
`Client registration blocked: private/reserved IP not allowed: ${registrationUrl}`,
126+
);
127+
}
128+
const pinnedIp = resolvedAddrs[0];
129+
const dispatcher = new Agent({
130+
connect: {
131+
lookup: (
132+
_hostname: string,
133+
_options: unknown,
134+
callback: (
135+
err: NodeJS.ErrnoException | null,
136+
addresses: Array<{ address: string; family: number }>,
137+
) => void,
138+
) => {
139+
callback(null, [
140+
{ address: pinnedIp, family: pinnedIp.includes(':') ? 6 : 4 },
141+
]);
142+
},
143+
},
144+
});
145+
114146
const response = await fetch(registrationUrl, {
115147
method: 'POST',
116148
headers: {
117149
'Content-Type': 'application/json',
118150
},
119151
body: JSON.stringify(registrationRequest),
120-
});
152+
dispatcher: dispatcher as Dispatcher,
153+
} as RequestInit & { dispatcher: Dispatcher });
121154

122155
if (!response.ok) {
123156
const errorText = await response.text();

‎packages/core/src/mcp/oauth-utils.ts‎

Lines changed: 70 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@
77
import type { MCPOAuthConfig } from './oauth-provider.js';
88
import { getErrorMessage } from '../utils/errors.js';
99
import { debugLogger } from '../utils/debugLogger.js';
10+
import { isLoopbackHost, resolveAndValidateDns } from '../utils/fetch.js';
11+
import { Agent, type Dispatcher } from 'undici';
1012

1113
/**
1214
* Error thrown when the discovered resource metadata does not match the expected resource.
@@ -97,7 +99,40 @@ export class OAuthUtils {
9799
resourceMetadataUrl: string,
98100
): Promise<OAuthProtectedResourceMetadata | null> {
99101
try {
100-
const response = await fetch(resourceMetadataUrl);
102+
const parsedUrl = new URL(resourceMetadataUrl);
103+
if (isLoopbackHost(parsedUrl.hostname)) {
104+
debugLogger.debug(
105+
`Blocked OAuth metadata fetch to loopback address: ${resourceMetadataUrl}`,
106+
);
107+
return null;
108+
}
109+
const resolvedAddrs = await resolveAndValidateDns(resourceMetadataUrl);
110+
if (resolvedAddrs.length === 0) {
111+
debugLogger.debug(
112+
`Blocked OAuth metadata fetch to private/reserved IP: ${resourceMetadataUrl}`,
113+
);
114+
return null;
115+
}
116+
const pinnedIp = resolvedAddrs[0];
117+
const dispatcher = new Agent({
118+
connect: {
119+
lookup: (
120+
_hostname: string,
121+
_options: unknown,
122+
callback: (
123+
err: NodeJS.ErrnoException | null,
124+
addresses: Array<{ address: string; family: number }>,
125+
) => void,
126+
) => {
127+
callback(null, [
128+
{ address: pinnedIp, family: pinnedIp.includes(':') ? 6 : 4 },
129+
]);
130+
},
131+
},
132+
});
133+
const response = await fetch(resourceMetadataUrl, {
134+
dispatcher: dispatcher as Dispatcher,
135+
} as RequestInit & { dispatcher: Dispatcher });
101136
if (!response.ok) {
102137
return null;
103138
}
@@ -121,7 +156,40 @@ export class OAuthUtils {
121156
authServerMetadataUrl: string,
122157
): Promise<OAuthAuthorizationServerMetadata | null> {
123158
try {
124-
const response = await fetch(authServerMetadataUrl);
159+
const parsedUrl = new URL(authServerMetadataUrl);
160+
if (isLoopbackHost(parsedUrl.hostname)) {
161+
debugLogger.debug(
162+
`Blocked OAuth metadata fetch to loopback address: ${authServerMetadataUrl}`,
163+
);
164+
return null;
165+
}
166+
const resolvedAddrs = await resolveAndValidateDns(authServerMetadataUrl);
167+
if (resolvedAddrs.length === 0) {
168+
debugLogger.debug(
169+
`Blocked OAuth metadata fetch to private/reserved IP: ${authServerMetadataUrl}`,
170+
);
171+
return null;
172+
}
173+
const pinnedIp = resolvedAddrs[0];
174+
const dispatcher = new Agent({
175+
connect: {
176+
lookup: (
177+
_hostname: string,
178+
_options: unknown,
179+
callback: (
180+
err: NodeJS.ErrnoException | null,
181+
addresses: Array<{ address: string; family: number }>,
182+
) => void,
183+
) => {
184+
callback(null, [
185+
{ address: pinnedIp, family: pinnedIp.includes(':') ? 6 : 4 },
186+
]);
187+
},
188+
},
189+
});
190+
const response = await fetch(authServerMetadataUrl, {
191+
dispatcher: dispatcher as Dispatcher,
192+
} as RequestInit & { dispatcher: Dispatcher });
125193
if (!response.ok) {
126194
return null;
127195
}

‎packages/core/src/tools/web-fetch.test.ts‎

Lines changed: 5 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -70,16 +70,9 @@ const mockFetch = (url: string, response: Partial<Response> | Error) =>
7070
vi
7171
.spyOn(fetchUtils, 'fetchWithTimeout')
7272
.mockImplementation(async (actualUrl) => {
73-
let pinnedUrlStr = url;
74-
try {
75-
const u = new URL(url);
76-
u.hostname = '8.8.8.8';
77-
pinnedUrlStr = u.toString();
78-
} catch {
79-
// ignore error
80-
}
81-
82-
if (actualUrl !== url && actualUrl !== pinnedUrlStr) {
73+
// With the undici Agent dispatcher pattern, the original URL is
74+
// always passed unchanged; DNS pinning happens inside the Agent.
75+
if (actualUrl !== url) {
8376
throw new Error(
8477
`Unexpected fetch URL: expected "${url}", got "${actualUrl}"`,
8578
);
@@ -1018,9 +1011,10 @@ describe('WebFetchTool', () => {
10181011
);
10191012
expect(result.returnDisplay).toContain('Fetched text/plain content');
10201013
expect(fetchUtils.fetchWithTimeout).toHaveBeenCalledWith(
1021-
'https://8.8.8.8/',
1014+
'https://example.com/',
10221015
expect.any(Number),
10231016
expect.objectContaining({
1017+
dispatcher: expect.any(Object),
10241018
headers: expect.objectContaining({
10251019
Accept: expect.stringContaining('text/plain'),
10261020
}),

‎packages/core/src/tools/web-fetch.ts‎

Lines changed: 47 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import {
2424
isLoopbackHost,
2525
resolveAndValidateDns,
2626
} from '../utils/fetch.js';
27+
import { Agent, type Dispatcher } from 'undici';
2728
import { truncateString, wrapUntrusted } from '../utils/textUtils.js';
2829
import { convert } from 'html-to-text';
2930
import {
@@ -315,23 +316,32 @@ class WebFetchToolInvocation extends BaseToolInvocation<
315316
);
316317
}
317318

318-
const urlWithIp = new URL(url);
319-
const originalHostname = urlWithIp.hostname;
320-
urlWithIp.hostname = pinnedIp;
319+
const dispatcher = new Agent({
320+
connect: {
321+
lookup: (
322+
_hostname: string,
323+
_options: unknown,
324+
callback: (
325+
err: NodeJS.ErrnoException | null,
326+
addresses: Array<{ address: string; family: number }>,
327+
) => void,
328+
) => {
329+
callback(null, [
330+
{ address: pinnedIp, family: pinnedIp.includes(':') ? 6 : 4 },
331+
]);
332+
},
333+
},
334+
}) as Dispatcher;
321335

322336
const response = await retryWithBackoff(
323337
async () => {
324-
const res = await fetchWithTimeout(
325-
urlWithIp.toString(),
326-
URL_FETCH_TIMEOUT_MS,
327-
{
328-
signal,
329-
headers: {
330-
'User-Agent': USER_AGENT,
331-
Host: originalHostname,
332-
},
338+
const res = await fetchWithTimeout(url, URL_FETCH_TIMEOUT_MS, {
339+
signal,
340+
dispatcher,
341+
headers: {
342+
'User-Agent': USER_AGENT,
333343
},
334-
);
344+
});
335345
if (!res.ok) {
336346
const error = new Error(
337347
`Request failed with status code ${res.status} ${res.statusText}`,
@@ -664,25 +674,34 @@ ${aggregatedContent}
664674
}
665675

666676
try {
667-
const urlWithIp = new URL(url);
668-
const originalHostname = urlWithIp.hostname;
669-
urlWithIp.hostname = pinnedIp;
677+
const dispatcher = new Agent({
678+
connect: {
679+
lookup: (
680+
_hostname: string,
681+
_options: unknown,
682+
callback: (
683+
err: NodeJS.ErrnoException | null,
684+
addresses: Array<{ address: string; family: number }>,
685+
) => void,
686+
) => {
687+
callback(null, [
688+
{ address: pinnedIp, family: pinnedIp.includes(':') ? 6 : 4 },
689+
]);
690+
},
691+
},
692+
}) as Dispatcher;
670693

671694
const response = await retryWithBackoff(
672695
async () => {
673-
const res = await fetchWithTimeout(
674-
urlWithIp.toString(),
675-
URL_FETCH_TIMEOUT_MS,
676-
{
677-
signal,
678-
headers: {
679-
Accept:
680-
'text/markdown, text/plain;q=0.9, application/json;q=0.9, text/html;q=0.8, application/pdf;q=0.7, video/*;q=0.7, */*;q=0.5',
681-
'User-Agent': USER_AGENT,
682-
Host: originalHostname,
683-
},
696+
const res = await fetchWithTimeout(url, URL_FETCH_TIMEOUT_MS, {
697+
signal,
698+
dispatcher,
699+
headers: {
700+
Accept:
701+
'text/markdown, text/plain;q=0.9, application/json;q=0.9, text/html;q=0.8, application/pdf;q=0.7, video/*;q=0.7, */*;q=0.5',
702+
'User-Agent': USER_AGENT,
684703
},
685-
);
704+
});
686705
return res;
687706
},
688707
{

‎packages/core/src/utils/fetch.ts‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,13 @@
66

77
import { getErrorMessage, isAbortError } from './errors.js';
88
import { URL } from 'node:url';
9-
import { Agent, EnvHttpProxyAgent, setGlobalDispatcher } from 'undici';
9+
import type {
10+
Dispatcher} from 'undici';
11+
import {
12+
Agent,
13+
EnvHttpProxyAgent,
14+
setGlobalDispatcher,
15+
} from 'undici';
1016
import ipaddr from 'ipaddr.js';
1117
import { lookup } from 'node:dns/promises';
1218

@@ -188,7 +194,7 @@ export function createSafeProxyAgent(proxyUrl: string): EnvHttpProxyAgent {
188194
export async function fetchWithTimeout(
189195
url: string,
190196
timeout: number,
191-
options?: RequestInit,
197+
options?: RequestInit & { dispatcher?: Dispatcher },
192198
): Promise<Response> {
193199
const controller = new AbortController();
194200
const timeoutId = setTimeout(() => controller.abort(), timeout);

0 commit comments

Comments
 (0)