Skip to content

Commit dc55d32

Browse files
committed
fix: prevent command injection in example URL opening
Replace exec() with execFile() and add URL scheme validation in both elicitationUrlExample.ts and simpleOAuthClient.ts. The previous code used exec() with string interpolation, which invokes a shell and allows command injection via crafted URLs containing shell metacharacters (e.g., double-quote escapes and & as command separators). Changes: - Use execFile() with array arguments instead of exec() with string interpolation to avoid shell interpretation - Add cross-platform support (open/xdg-open/start) instead of hardcoding macOS open command - Add URL scheme allowlist (http/https only) to prevent abuse via dangerous protocol handlers (file://, smb://, ms-msdt://, etc.)
1 parent d6dc0ab commit dc55d32

4 files changed

Lines changed: 134 additions & 17 deletions

File tree

examples/client/package.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,12 +36,13 @@
3636
"dependencies": {
3737
"@modelcontextprotocol/client": "workspace:^",
3838
"ajv": "catalog:runtimeShared",
39+
"open": "^11.0.0",
3940
"zod": "catalog:runtimeShared"
4041
},
4142
"devDependencies": {
43+
"@modelcontextprotocol/eslint-config": "workspace:^",
4244
"@modelcontextprotocol/examples-shared": "workspace:^",
4345
"@modelcontextprotocol/tsconfig": "workspace:^",
44-
"@modelcontextprotocol/eslint-config": "workspace:^",
4546
"@modelcontextprotocol/vitest-config": "workspace:^",
4647
"tsdown": "catalog:devTools"
4748
}

examples/client/src/elicitationUrlExample.ts

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55
// URL elicitation allows servers to prompt the end-user to open a URL in their browser
66
// to collect sensitive information.
77

8-
import { exec } from 'node:child_process';
98
import { createServer } from 'node:http';
109
import { createInterface } from 'node:readline';
1110

@@ -29,6 +28,7 @@ import {
2928
UnauthorizedError,
3029
UrlElicitationRequiredError
3130
} from '@modelcontextprotocol/client';
31+
import open from 'open';
3232

3333
import { InMemoryOAuthClientProvider } from './simpleOAuthClientProvider.js';
3434

@@ -272,15 +272,25 @@ async function elicitationLoop(): Promise<void> {
272272
}
273273
}
274274

275-
async function openBrowser(url: string): Promise<void> {
276-
const command = `open "${url}"`;
275+
const ALLOWED_SCHEMES = new Set(['http:', 'https:']);
277276

278-
exec(command, error => {
279-
if (error) {
280-
console.error(`Failed to open browser: ${error.message}`);
281-
console.log(`Please manually open: ${url}`);
277+
async function openBrowser(url: string): Promise<void> {
278+
try {
279+
const parsed = new URL(url);
280+
if (!ALLOWED_SCHEMES.has(parsed.protocol)) {
281+
console.error(`Refusing to open URL with unsupported scheme '${parsed.protocol}': ${url}`);
282+
return;
282283
}
283-
});
284+
} catch {
285+
console.error(`Invalid URL: ${url}`);
286+
return;
287+
}
288+
289+
try {
290+
await open(url);
291+
} catch {
292+
console.log(`Please manually open: ${url}`);
293+
}
284294
}
285295

286296
/**

examples/client/src/simpleOAuthClient.ts

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
#!/usr/bin/env node
22

3-
import { exec } from 'node:child_process';
43
import { createServer } from 'node:http';
54
import { createInterface } from 'node:readline';
65
import { URL } from 'node:url';
@@ -13,6 +12,7 @@ import {
1312
StreamableHTTPClientTransport,
1413
UnauthorizedError
1514
} from '@modelcontextprotocol/client';
15+
import open from 'open';
1616

1717
import { InMemoryOAuthClientProvider } from './simpleOAuthClientProvider.js';
1818

@@ -49,17 +49,27 @@ class InteractiveOAuthClient {
4949
/**
5050
* Opens the authorization URL in the user's default browser
5151
*/
52+
private static readonly ALLOWED_SCHEMES = new Set(['http:', 'https:']);
53+
5254
private async openBrowser(url: string): Promise<void> {
5355
console.log(`🌐 Opening browser for authorization: ${url}`);
5456

55-
const command = `open "${url}"`;
56-
57-
exec(command, error => {
58-
if (error) {
59-
console.error(`Failed to open browser: ${error.message}`);
60-
console.log(`Please manually open: ${url}`);
57+
try {
58+
const parsed = new URL(url);
59+
if (!InteractiveOAuthClient.ALLOWED_SCHEMES.has(parsed.protocol)) {
60+
console.error(`Refusing to open URL with unsupported scheme '${parsed.protocol}': ${url}`);
61+
return;
6162
}
62-
});
63+
} catch {
64+
console.error(`Invalid URL: ${url}`);
65+
return;
66+
}
67+
68+
try {
69+
await open(url);
70+
} catch {
71+
console.log(`Please manually open: ${url}`);
72+
}
6373
}
6474
/**
6575
* Example OAuth callback handler - in production, use a more robust approach

pnpm-lock.yaml

Lines changed: 96 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)