mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-03-13 11:26:18 +01:00
🔒 fix: Hex-normalized IPv4-mapped IPv6 in Domain Validation (#12130)
Some checks are pending
Docker Dev Branch Images Build / build (Dockerfile, lc-dev, node) (push) Waiting to run
Docker Dev Branch Images Build / build (Dockerfile.multi, lc-dev-api, api-build) (push) Waiting to run
Docker Dev Images Build / build (Dockerfile, librechat-dev, node) (push) Waiting to run
Docker Dev Images Build / build (Dockerfile.multi, librechat-dev-api, api-build) (push) Waiting to run
Sync Locize Translations & Create Translation PR / Sync Translation Keys with Locize (push) Waiting to run
Sync Locize Translations & Create Translation PR / Create Translation PR on Version Published (push) Blocked by required conditions
Some checks are pending
Docker Dev Branch Images Build / build (Dockerfile, lc-dev, node) (push) Waiting to run
Docker Dev Branch Images Build / build (Dockerfile.multi, lc-dev-api, api-build) (push) Waiting to run
Docker Dev Images Build / build (Dockerfile, librechat-dev, node) (push) Waiting to run
Docker Dev Images Build / build (Dockerfile.multi, librechat-dev-api, api-build) (push) Waiting to run
Sync Locize Translations & Create Translation PR / Sync Translation Keys with Locize (push) Waiting to run
Sync Locize Translations & Create Translation PR / Create Translation PR on Version Published (push) Blocked by required conditions
* 🔒 fix: handle hex-normalized IPv4-mapped IPv6 in domain validation
* fix: Enhance IPv6 private address detection in domain validation
- Added tests for detecting IPv4-compatible, 6to4, NAT64, and Teredo addresses.
- Implemented `extractEmbeddedIPv4` function to identify private IPv4 addresses within various IPv6 formats.
- Updated `isPrivateIP` function to utilize the new extraction logic for improved accuracy in address validation.
* fix: Update private IPv4 detection logic in domain validation
- Enhanced the `isPrivateIPv4` function to accurately identify additional private and non-routable IPv4 ranges.
- Adjusted the return logic in `resolveHostnameSSRF` to utilize the updated private IP detection for improved hostname validation.
* test: Expand private IP detection tests in domain validation
- Added tests for additional private IPv4 ranges including 0.0.0.0/8, 100.64.0.0/10, 192.0.0.0/24, and 198.18.0.0/15.
- Updated existing tests to ensure accurate detection of private and multicast IP addresses in the `isPrivateIP` function.
- Enhanced `resolveHostnameSSRF` to correctly identify private literal IPv4 addresses without DNS lookup.
* refactor: Rename and enhance embedded IPv4 detection in IPv6 addresses
- Renamed `extractEmbeddedIPv4` to `hasPrivateEmbeddedIPv4` for clarity on its purpose.
- Updated logic to accurately check for private IPv4 addresses embedded in Teredo, 6to4, and NAT64 IPv6 formats.
- Improved the `isPrivateIP` function to utilize the new naming and logic for better readability and accuracy.
- Enhanced documentation for clarity on the functionality of the updated methods.
* feat: Enhance private IPv4 detection in embedded IPv6 addresses
- Added additional checks in `hasPrivateEmbeddedIPv4` to ensure only valid private IPv4 formats are recognized.
- Improved the logic for identifying private IPv4 addresses embedded within various IPv6 formats, enhancing overall accuracy.
* test: Add additional test for hostname resolution in SSRF detection
- Included a new test case in `resolveHostnameSSRF` to validate the detection of private IPv4 addresses embedded in IPv6 formats for the hostname 'meta.example.com'.
- Enhanced existing tests to ensure comprehensive coverage of hostname resolution scenarios.
* fix: Set redirect option to 'manual' in undiciFetch calls
- Updated undiciFetch calls in MCPConnection to include the redirect option set to 'manual' for better control over HTTP redirects.
- Added documentation comments regarding SSRF pre-checks for WebSocket connections, highlighting the limitations of the current SDK regarding DNS resolution.
* test: Add integration tests for MCP SSRF protections
- Introduced a new test suite for MCP SSRF protections, verifying that MCPConnection does not follow HTTP redirects to private IPs and blocks WebSocket connections to private IPs when SSRF protection is enabled.
- Implemented tests to ensure correct behavior of the connection under various scenarios, including redirect handling and WebSocket DNS resolution.
* refactor: Improve SSRF protection logic for WebSocket connections
- Enhanced the SSRF pre-check for WebSocket connections to validate resolved IPs, ensuring that allowlisting a domain does not grant trust to its resolved IPs at runtime.
- Updated documentation comments to clarify the limitations of the current SDK regarding DNS resolution and the implications for SSRF protection.
* test: Enhance MCP SSRF protection tests for redirect handling and WebSocket connections
- Updated tests to ensure that MCPConnection does not follow HTTP redirects to private IPs, regardless of SSRF protection settings.
- Added checks to verify that WebSocket connections to hosts resolving to private IPs are blocked, even when SSRF protection is disabled.
- Improved documentation comments for clarity on the behavior of the tests and the implications for SSRF protection.
* test: Refactor MCP SSRF protection test for WebSocket connection errors
- Updated the test to use `await expect(...).rejects.not.toThrow(...)` for better readability and clarity.
- Simplified the error handling logic while ensuring that SSRF rejections are correctly validated during connection failures.
This commit is contained in:
parent
2ac62a2e71
commit
4a8a5b5994
4 changed files with 627 additions and 33 deletions
277
packages/api/src/mcp/__tests__/MCPConnectionSSRF.test.ts
Normal file
277
packages/api/src/mcp/__tests__/MCPConnectionSSRF.test.ts
Normal file
|
|
@ -0,0 +1,277 @@
|
|||
/**
|
||||
* Integration tests for MCP SSRF protections.
|
||||
*
|
||||
* These tests spin up real in-process HTTP servers and verify that MCPConnection:
|
||||
*
|
||||
* 1. Does NOT follow HTTP redirects from SSE/StreamableHTTP transports
|
||||
* (redirect: 'manual' prevents SSRF via server-controlled 301/302)
|
||||
* 2. Blocks WebSocket connections to hosts that DNS-resolve to private IPs,
|
||||
* regardless of whether useSSRFProtection is enabled (allowlist scenario)
|
||||
*/
|
||||
|
||||
import * as net from 'net';
|
||||
import * as http from 'http';
|
||||
import { randomUUID } from 'crypto';
|
||||
import { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js';
|
||||
import { StreamableHTTPServerTransport } from '@modelcontextprotocol/sdk/server/streamableHttp.js';
|
||||
import type { Socket } from 'net';
|
||||
import { MCPConnection } from '~/mcp/connection';
|
||||
import { resolveHostnameSSRF } from '~/auth';
|
||||
|
||||
jest.mock('@librechat/data-schemas', () => ({
|
||||
logger: {
|
||||
info: jest.fn(),
|
||||
warn: jest.fn(),
|
||||
error: jest.fn(),
|
||||
debug: jest.fn(),
|
||||
},
|
||||
}));
|
||||
|
||||
jest.mock('~/auth', () => ({
|
||||
createSSRFSafeUndiciConnect: jest.fn(() => undefined),
|
||||
resolveHostnameSSRF: jest.fn(async () => false),
|
||||
}));
|
||||
|
||||
jest.mock('~/mcp/mcpConfig', () => ({
|
||||
mcpConfig: { CONNECTION_CHECK_TTL: 0 },
|
||||
}));
|
||||
|
||||
const mockedResolveHostnameSSRF = resolveHostnameSSRF as jest.MockedFunction<
|
||||
typeof resolveHostnameSSRF
|
||||
>;
|
||||
|
||||
async function safeDisconnect(conn: MCPConnection | null): Promise<void> {
|
||||
if (!conn) {
|
||||
return;
|
||||
}
|
||||
(conn as unknown as { shouldStopReconnecting: boolean }).shouldStopReconnecting = true;
|
||||
conn.removeAllListeners();
|
||||
await conn.disconnect();
|
||||
}
|
||||
|
||||
interface TestServer {
|
||||
url: string;
|
||||
redirectHit: boolean;
|
||||
close: () => Promise<void>;
|
||||
}
|
||||
|
||||
function getFreePort(): Promise<number> {
|
||||
return new Promise((resolve, reject) => {
|
||||
const srv = net.createServer();
|
||||
srv.listen(0, '127.0.0.1', () => {
|
||||
const addr = srv.address() as net.AddressInfo;
|
||||
srv.close((err) => (err ? reject(err) : resolve(addr.port)));
|
||||
});
|
||||
});
|
||||
}
|
||||
|
||||
function trackSockets(httpServer: http.Server): () => Promise<void> {
|
||||
const sockets = new Set<Socket>();
|
||||
httpServer.on('connection', (socket: Socket) => {
|
||||
sockets.add(socket);
|
||||
socket.once('close', () => sockets.delete(socket));
|
||||
});
|
||||
return () =>
|
||||
new Promise<void>((resolve) => {
|
||||
for (const socket of sockets) {
|
||||
socket.destroy();
|
||||
}
|
||||
sockets.clear();
|
||||
httpServer.close(() => resolve());
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* Creates an HTTP server that responds with a 301 redirect to a target URL.
|
||||
* A second server is spun up at the redirect target to detect whether the
|
||||
* redirect was actually followed.
|
||||
*/
|
||||
async function createRedirectingServer(redirectTarget: string): Promise<TestServer> {
|
||||
const state = { redirectHit: false };
|
||||
|
||||
const targetPort = new URL(redirectTarget).port || '80';
|
||||
const targetServer = http.createServer((_req, res) => {
|
||||
state.redirectHit = true;
|
||||
res.writeHead(200, { 'Content-Type': 'text/plain' });
|
||||
res.end('You should not be here');
|
||||
});
|
||||
const destroyTargetSockets = trackSockets(targetServer);
|
||||
await new Promise<void>((resolve) =>
|
||||
targetServer.listen(parseInt(targetPort), '127.0.0.1', resolve),
|
||||
);
|
||||
|
||||
const httpServer = http.createServer((_req, res) => {
|
||||
res.writeHead(301, { Location: redirectTarget });
|
||||
res.end();
|
||||
});
|
||||
|
||||
const destroySockets = trackSockets(httpServer);
|
||||
const port = await getFreePort();
|
||||
await new Promise<void>((resolve) => httpServer.listen(port, '127.0.0.1', resolve));
|
||||
|
||||
return {
|
||||
url: `http://127.0.0.1:${port}/`,
|
||||
get redirectHit() {
|
||||
return state.redirectHit;
|
||||
},
|
||||
close: async () => {
|
||||
await destroySockets();
|
||||
await destroyTargetSockets();
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* Creates a real StreamableHTTP MCP server for baseline connectivity tests.
|
||||
*/
|
||||
async function createStreamableServer(): Promise<Omit<TestServer, 'redirectHit'>> {
|
||||
const sessions = new Map<string, StreamableHTTPServerTransport>();
|
||||
|
||||
const httpServer = http.createServer(async (req, res) => {
|
||||
const sid = req.headers['mcp-session-id'] as string | undefined;
|
||||
let transport = sid ? sessions.get(sid) : undefined;
|
||||
|
||||
if (!transport) {
|
||||
transport = new StreamableHTTPServerTransport({ sessionIdGenerator: () => randomUUID() });
|
||||
const mcp = new McpServer({ name: 'test-ssrf', version: '0.0.1' });
|
||||
await mcp.connect(transport);
|
||||
}
|
||||
|
||||
await transport.handleRequest(req, res);
|
||||
|
||||
if (transport.sessionId && !sessions.has(transport.sessionId)) {
|
||||
sessions.set(transport.sessionId, transport);
|
||||
transport.onclose = () => sessions.delete(transport!.sessionId!);
|
||||
}
|
||||
});
|
||||
|
||||
const destroySockets = trackSockets(httpServer);
|
||||
const port = await getFreePort();
|
||||
await new Promise<void>((resolve) => httpServer.listen(port, '127.0.0.1', resolve));
|
||||
|
||||
return {
|
||||
url: `http://127.0.0.1:${port}/`,
|
||||
close: async () => {
|
||||
const closing = [...sessions.values()].map((t) => t.close().catch(() => undefined));
|
||||
sessions.clear();
|
||||
await Promise.all(closing);
|
||||
await destroySockets();
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
describe('MCP SSRF protection – redirect blocking', () => {
|
||||
let redirectServer: TestServer;
|
||||
let conn: MCPConnection | null;
|
||||
|
||||
afterEach(async () => {
|
||||
await safeDisconnect(conn);
|
||||
conn = null;
|
||||
if (redirectServer) {
|
||||
await redirectServer.close();
|
||||
}
|
||||
jest.restoreAllMocks();
|
||||
});
|
||||
|
||||
it('should not follow redirects from streamable-http to a private IP', async () => {
|
||||
const targetPort = await getFreePort();
|
||||
redirectServer = await createRedirectingServer(
|
||||
`http://127.0.0.1:${targetPort}/latest/meta-data/`,
|
||||
);
|
||||
|
||||
conn = new MCPConnection({
|
||||
serverName: 'redirect-test',
|
||||
serverConfig: { type: 'streamable-http', url: redirectServer.url },
|
||||
useSSRFProtection: false,
|
||||
});
|
||||
|
||||
await expect(conn.connect()).rejects.toThrow();
|
||||
expect(redirectServer.redirectHit).toBe(false);
|
||||
});
|
||||
|
||||
it('should not follow redirects even with SSRF protection off (allowlist scenario)', async () => {
|
||||
const targetPort = await getFreePort();
|
||||
redirectServer = await createRedirectingServer(`http://127.0.0.1:${targetPort}/admin`);
|
||||
|
||||
conn = new MCPConnection({
|
||||
serverName: 'redirect-test-2',
|
||||
serverConfig: { type: 'streamable-http', url: redirectServer.url },
|
||||
useSSRFProtection: false,
|
||||
});
|
||||
|
||||
await expect(conn.connect()).rejects.toThrow();
|
||||
expect(redirectServer.redirectHit).toBe(false);
|
||||
});
|
||||
|
||||
it('should connect normally to a non-redirecting streamable-http server', async () => {
|
||||
const realServer = await createStreamableServer();
|
||||
try {
|
||||
conn = new MCPConnection({
|
||||
serverName: 'legit-server',
|
||||
serverConfig: { type: 'streamable-http', url: realServer.url },
|
||||
useSSRFProtection: false,
|
||||
});
|
||||
|
||||
await conn.connect();
|
||||
const tools = await conn.fetchTools();
|
||||
expect(tools).toBeDefined();
|
||||
} finally {
|
||||
await safeDisconnect(conn);
|
||||
conn = null;
|
||||
await realServer.close();
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe('MCP SSRF protection – WebSocket DNS resolution', () => {
|
||||
let conn: MCPConnection | null;
|
||||
|
||||
afterEach(async () => {
|
||||
await safeDisconnect(conn);
|
||||
conn = null;
|
||||
jest.restoreAllMocks();
|
||||
});
|
||||
|
||||
it('should block WebSocket to host resolving to private IP when SSRF protection is on', async () => {
|
||||
mockedResolveHostnameSSRF.mockResolvedValueOnce(true);
|
||||
|
||||
conn = new MCPConnection({
|
||||
serverName: 'ws-ssrf-test',
|
||||
serverConfig: { type: 'websocket', url: 'ws://evil.example.com:8080/mcp' },
|
||||
useSSRFProtection: true,
|
||||
});
|
||||
|
||||
await expect(conn.connect()).rejects.toThrow(/SSRF protection/);
|
||||
expect(mockedResolveHostnameSSRF).toHaveBeenCalledWith(
|
||||
expect.stringContaining('evil.example.com'),
|
||||
);
|
||||
});
|
||||
|
||||
it('should block WebSocket to host resolving to private IP even with SSRF protection off', async () => {
|
||||
mockedResolveHostnameSSRF.mockResolvedValueOnce(true);
|
||||
|
||||
conn = new MCPConnection({
|
||||
serverName: 'ws-ssrf-allowlist',
|
||||
serverConfig: { type: 'websocket', url: 'ws://allowlisted.example.com:8080/mcp' },
|
||||
useSSRFProtection: false,
|
||||
});
|
||||
|
||||
await expect(conn.connect()).rejects.toThrow(/SSRF protection/);
|
||||
expect(mockedResolveHostnameSSRF).toHaveBeenCalledWith(
|
||||
expect.stringContaining('allowlisted.example.com'),
|
||||
);
|
||||
});
|
||||
|
||||
it('should allow WebSocket to host resolving to public IP', async () => {
|
||||
mockedResolveHostnameSSRF.mockResolvedValueOnce(false);
|
||||
|
||||
conn = new MCPConnection({
|
||||
serverName: 'ws-public-test',
|
||||
serverConfig: { type: 'websocket', url: 'ws://public.example.com:8080/mcp' },
|
||||
useSSRFProtection: true,
|
||||
});
|
||||
|
||||
/** Fails on connect (no real server), but the error must not be an SSRF rejection. */
|
||||
await expect(conn.connect()).rejects.not.toThrow(/SSRF protection/);
|
||||
});
|
||||
});
|
||||
|
|
@ -364,7 +364,7 @@ export class MCPConnection extends EventEmitter {
|
|||
|
||||
const requestHeaders = getHeaders();
|
||||
if (!requestHeaders) {
|
||||
return undiciFetch(input, { ...init, dispatcher });
|
||||
return undiciFetch(input, { ...init, redirect: 'manual', dispatcher });
|
||||
}
|
||||
|
||||
let initHeaders: Record<string, string> = {};
|
||||
|
|
@ -380,6 +380,7 @@ export class MCPConnection extends EventEmitter {
|
|||
|
||||
return undiciFetch(input, {
|
||||
...init,
|
||||
redirect: 'manual',
|
||||
headers: {
|
||||
...initHeaders,
|
||||
...requestHeaders,
|
||||
|
|
@ -425,21 +426,29 @@ export class MCPConnection extends EventEmitter {
|
|||
env: { ...getDefaultEnvironment(), ...(options.env ?? {}) },
|
||||
});
|
||||
|
||||
case 'websocket':
|
||||
case 'websocket': {
|
||||
if (!isWebSocketOptions(options)) {
|
||||
throw new Error('Invalid options for websocket transport.');
|
||||
}
|
||||
this.url = options.url;
|
||||
if (this.useSSRFProtection) {
|
||||
const wsHostname = new URL(options.url).hostname;
|
||||
const isSSRF = await resolveHostnameSSRF(wsHostname);
|
||||
if (isSSRF) {
|
||||
throw new Error(
|
||||
`SSRF protection: WebSocket host "${wsHostname}" resolved to a private/reserved IP address`,
|
||||
);
|
||||
}
|
||||
/**
|
||||
* SSRF pre-check: always validate resolved IPs for WebSocket, regardless
|
||||
* of allowlist configuration. Allowlisting a domain grants trust to that
|
||||
* name, not to whatever IP it resolves to at runtime (DNS rebinding).
|
||||
*
|
||||
* Note: WebSocketClientTransport does its own DNS resolution, creating a
|
||||
* small TOCTOU window. This is an SDK limitation — the transport accepts
|
||||
* only a URL with no custom DNS lookup hook.
|
||||
*/
|
||||
const wsHostname = new URL(options.url).hostname;
|
||||
const isSSRF = await resolveHostnameSSRF(wsHostname);
|
||||
if (isSSRF) {
|
||||
throw new Error(
|
||||
`SSRF protection: WebSocket host "${wsHostname}" resolved to a private/reserved IP address`,
|
||||
);
|
||||
}
|
||||
return new WebSocketClientTransport(new URL(options.url));
|
||||
}
|
||||
|
||||
case 'sse': {
|
||||
if (!isSSEOptions(options)) {
|
||||
|
|
@ -486,6 +495,7 @@ export class MCPConnection extends EventEmitter {
|
|||
);
|
||||
return undiciFetch(url, {
|
||||
...init,
|
||||
redirect: 'manual',
|
||||
dispatcher: sseAgent,
|
||||
headers: fetchHeaders,
|
||||
});
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue