From dc55d329886b0b9255aed4b9cab358dbc27ab2db Mon Sep 17 00:00:00 2001 From: Max Isbey <224885523+maxisbey@users.noreply.github.com> Date: Wed, 18 Feb 2026 14:49:14 +0000 Subject: [PATCH] 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.) --- examples/client/package.json | 3 +- examples/client/src/elicitationUrlExample.ts | 26 ++++-- examples/client/src/simpleOAuthClient.ts | 26 ++++-- pnpm-lock.yaml | 96 ++++++++++++++++++++ 4 files changed, 134 insertions(+), 17 deletions(-) diff --git a/examples/client/package.json b/examples/client/package.json index d77d6faf2..c5ea6be9d 100644 --- a/examples/client/package.json +++ b/examples/client/package.json @@ -36,12 +36,13 @@ "dependencies": { "@modelcontextprotocol/client": "workspace:^", "ajv": "catalog:runtimeShared", + "open": "^11.0.0", "zod": "catalog:runtimeShared" }, "devDependencies": { + "@modelcontextprotocol/eslint-config": "workspace:^", "@modelcontextprotocol/examples-shared": "workspace:^", "@modelcontextprotocol/tsconfig": "workspace:^", - "@modelcontextprotocol/eslint-config": "workspace:^", "@modelcontextprotocol/vitest-config": "workspace:^", "tsdown": "catalog:devTools" } diff --git a/examples/client/src/elicitationUrlExample.ts b/examples/client/src/elicitationUrlExample.ts index f8ce2b1e1..dc6f634cc 100644 --- a/examples/client/src/elicitationUrlExample.ts +++ b/examples/client/src/elicitationUrlExample.ts @@ -5,7 +5,6 @@ // URL elicitation allows servers to prompt the end-user to open a URL in their browser // to collect sensitive information. -import { exec } from 'node:child_process'; import { createServer } from 'node:http'; import { createInterface } from 'node:readline'; @@ -29,6 +28,7 @@ import { UnauthorizedError, UrlElicitationRequiredError } from '@modelcontextprotocol/client'; +import open from 'open'; import { InMemoryOAuthClientProvider } from './simpleOAuthClientProvider.js'; @@ -272,15 +272,25 @@ async function elicitationLoop(): Promise { } } -async function openBrowser(url: string): Promise { - const command = `open "${url}"`; +const ALLOWED_SCHEMES = new Set(['http:', 'https:']); - exec(command, error => { - if (error) { - console.error(`Failed to open browser: ${error.message}`); - console.log(`Please manually open: ${url}`); +async function openBrowser(url: string): Promise { + try { + const parsed = new URL(url); + if (!ALLOWED_SCHEMES.has(parsed.protocol)) { + console.error(`Refusing to open URL with unsupported scheme '${parsed.protocol}': ${url}`); + return; } - }); + } catch { + console.error(`Invalid URL: ${url}`); + return; + } + + try { + await open(url); + } catch { + console.log(`Please manually open: ${url}`); + } } /** diff --git a/examples/client/src/simpleOAuthClient.ts b/examples/client/src/simpleOAuthClient.ts index 9ebc046b9..122d695d7 100644 --- a/examples/client/src/simpleOAuthClient.ts +++ b/examples/client/src/simpleOAuthClient.ts @@ -1,6 +1,5 @@ #!/usr/bin/env node -import { exec } from 'node:child_process'; import { createServer } from 'node:http'; import { createInterface } from 'node:readline'; import { URL } from 'node:url'; @@ -13,6 +12,7 @@ import { StreamableHTTPClientTransport, UnauthorizedError } from '@modelcontextprotocol/client'; +import open from 'open'; import { InMemoryOAuthClientProvider } from './simpleOAuthClientProvider.js'; @@ -49,17 +49,27 @@ class InteractiveOAuthClient { /** * Opens the authorization URL in the user's default browser */ + private static readonly ALLOWED_SCHEMES = new Set(['http:', 'https:']); + private async openBrowser(url: string): Promise { console.log(`🌐 Opening browser for authorization: ${url}`); - const command = `open "${url}"`; - - exec(command, error => { - if (error) { - console.error(`Failed to open browser: ${error.message}`); - console.log(`Please manually open: ${url}`); + try { + const parsed = new URL(url); + if (!InteractiveOAuthClient.ALLOWED_SCHEMES.has(parsed.protocol)) { + console.error(`Refusing to open URL with unsupported scheme '${parsed.protocol}': ${url}`); + return; } - }); + } catch { + console.error(`Invalid URL: ${url}`); + return; + } + + try { + await open(url); + } catch { + console.log(`Please manually open: ${url}`); + } } /** * Example OAuth callback handler - in production, use a more robust approach diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 3150f1648..cc778c3cf 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -292,6 +292,9 @@ importers: ajv: specifier: catalog:runtimeShared version: 8.17.1 + open: + specifier: ^11.0.0 + version: 11.0.0 zod: specifier: catalog:runtimeShared version: 4.3.5 @@ -2651,6 +2654,10 @@ packages: resolution: {integrity: sha512-bkXY9WsVpY7CvMhKSR6pZilZu9Ln5WDrKVBUXf2S443etkmEO4V58heTecXcUIsNsi4Rx8JUO4NfX1IcQl4deg==} engines: {node: '>=18.20'} + bundle-name@4.1.0: + resolution: {integrity: sha512-tjwM5exMg6BGRI+kNmTntNsvdZS1X8BFYS6tnJ2hdH0kVxM6/eVZ2xy+FqStSWvYmtfFMDLIxurorHwDKfDz5Q==} + engines: {node: '>=18'} + bytes@3.1.2: resolution: {integrity: sha512-/Nf7TyzTx6S3yRJObOAV7956r8cr2+Oj8AC5dt8wSP3BQAoeX58NoHyCU8P8zGkNXStjTSi6fzO6F0pBdcYbEg==} engines: {node: '>= 0.8'} @@ -2805,10 +2812,22 @@ packages: deep-is@0.1.4: resolution: {integrity: sha512-oIPzksmTg4/MriiaYGO+okXDT7ztn/w3Eptv/+gSIdMdKsJo0u4CfYNFJPy+4SKMuCqGw2wxnA+URMg3t8a/bQ==} + default-browser-id@5.0.1: + resolution: {integrity: sha512-x1VCxdX4t+8wVfd1so/9w+vQ4vx7lKd2Qp5tDRutErwmR85OgmfX7RlLRMWafRMY7hbEiXIbudNrjOAPa/hL8Q==} + engines: {node: '>=18'} + + default-browser@5.5.0: + resolution: {integrity: sha512-H9LMLr5zwIbSxrmvikGuI/5KGhZ8E2zH3stkMgM5LpOWDutGM2JZaj460Udnf1a+946zc7YBgrqEWwbk7zHvGw==} + engines: {node: '>=18'} + define-data-property@1.1.4: resolution: {integrity: sha512-rBMvIzlpA8v6E+SJZoo++HAYqsLrkg7MSfIinMPFhmkorw7X+dOXVJQs+QT69zGkzMyfDnIMN2Wid1+NbL3T+A==} engines: {node: '>= 0.4'} + define-lazy-prop@3.0.0: + resolution: {integrity: sha512-N+MeXYoqr3pOgn8xfyRPREN7gHakLYjhsHhWGT3fWAiL4IkAt0iDw14QiiEm2bE30c5XX5q0FtAA3CK5f9/BUg==} + engines: {node: '>=12'} + define-properties@1.2.1: resolution: {integrity: sha512-8QmQKqEASLd5nx0U1B1okLElbUuuttJ/AnYmRXbbbGDWh6uS208EjD4Xqq/I9wK7u0v6O08XhTWnt5XtEbR6Dg==} engines: {node: '>= 0.4'} @@ -3426,6 +3445,11 @@ packages: resolution: {integrity: sha512-PwwhEakHVKTdRNVOw+/Gyh0+MzlCl4R6qKvkhuvLtPMggI1WAHt9sOwZxQLSGpUaDnrdyDsomoRgNnCfKNSXXg==} engines: {node: '>= 0.4'} + is-docker@3.0.0: + resolution: {integrity: sha512-eljcgEDlEns/7AXFosB5K/2nCM4P7FQPkGc/DWLy5rmFEWvZayGrik1d9/QIY5nJ4f9YsVvBkA6kJpHn9rISdQ==} + engines: {node: ^12.20.0 || ^14.13.1 || >=16.0.0} + hasBin: true + is-extglob@2.1.1: resolution: {integrity: sha512-SbKbANkN603Vi4jEZv49LeVJMn4yGwsbzZworEoyEiutsN3nJYdbO36zfhGJ6QEDpOZIFkDtnq5JRxmvl3jsoQ==} engines: {node: '>=0.10.0'} @@ -3442,6 +3466,15 @@ packages: resolution: {integrity: sha512-xelSayHH36ZgE7ZWhli7pW34hNbNl8Ojv5KVmkJD4hBdD3th8Tfk9vYasLM+mXWOZhFkgZfxhLSnrwRr4elSSg==} engines: {node: '>=0.10.0'} + is-in-ssh@1.0.0: + resolution: {integrity: sha512-jYa6Q9rH90kR1vKB6NM7qqd1mge3Fx4Dhw5TVlK1MUBqhEOuCagrEHMevNuCcbECmXZ0ThXkRm+Ymr51HwEPAw==} + engines: {node: '>=20'} + + is-inside-container@1.0.0: + resolution: {integrity: sha512-KIYLCCJghfHZxqjYBE7rEy0OBuTd5xCHS7tHVgvCLkx7StIoaxwNW3hCALgEUjFfeRk+MG/Qxmp/vtETEF3tRA==} + engines: {node: '>=14.16'} + hasBin: true + is-map@2.0.3: resolution: {integrity: sha512-1Qed0/Hr2m+YqxnM09CjA2d/i6YZNfF6R2oRAOj36eUdS6qIV/huPJNSEpKbupewFs+ZsJlxsjjPbc0/afW6Lw==} engines: {node: '>= 0.4'} @@ -3505,6 +3538,10 @@ packages: resolution: {integrity: sha512-eXK1UInq2bPmjyX6e3VHIzMLobc4J94i4AWn+Hpq3OU5KkrRC96OAcR3PRJ/pGu6m8TRnBHP9dkXQVsT/COVIA==} engines: {node: '>=0.10.0'} + is-wsl@3.1.0: + resolution: {integrity: sha512-UcVfVfaK4Sc4m7X3dUSoHoozQGBEFeDC+zVo06t98xe8CzHSZZBekNXH+tu0NalHolcJ/QAGqS46Hef7QXBIMw==} + engines: {node: '>=16'} + isarray@2.0.5: resolution: {integrity: sha512-xHjhDr3cNBK0BzdUJSPXZntQUx/mwMS5Rw4A7lPJ90XGAO6ISP/ePDNuo0vhqOZU+UD5JoodwCAAoZQd3FeAKw==} @@ -3750,6 +3787,10 @@ packages: once@1.4.0: resolution: {integrity: sha512-lNaJgI+2Q5URQBkccEKHTQOPaXdUxnZZElQTZY0MFUAuaEqe1E+Nyvgdz/aIyNi6Z9MzO5dv1H8n58/GELp3+w==} + open@11.0.0: + resolution: {integrity: sha512-smsWv2LzFjP03xmvFoJ331ss6h+jixfA4UUV/Bsiyuu4YJPfN+FIQGOIiv4w9/+MoHkfkJ22UIaQWRVFRfH6Vw==} + engines: {node: '>=20'} + optionator@0.9.4: resolution: {integrity: sha512-6IpQ7mKUxRcZNLIObR0hz7lxsapSSIYNZJwXPGeF0mTVqGKFIXj1DQcMoT22S3ROcLyY/rz0PWaWZ9ayWmad9g==} engines: {node: '>= 0.8.0'} @@ -3855,6 +3896,10 @@ packages: resolution: {integrity: sha512-3Ybi1tAuwAP9s0r1UQ2J4n5Y0G05bJkpUIO0/bI9MhwmD70S5aTWbXGBwxHrelT+XM1k6dM0pk+SwNkpTRN7Pg==} engines: {node: ^10 || ^12 || >=14} + powershell-utils@0.1.0: + resolution: {integrity: sha512-dM0jVuXJPsDN6DvRpea484tCUaMiXWjuCn++HGTqUWzGDjv5tZkEZldAJ/UMlqRYGFrD/etByo4/xOuC/snX2A==} + engines: {node: '>=20'} + prebuild-install@7.1.3: resolution: {integrity: sha512-8Mf2cbV7x1cXPUILADGI3wuhfqWvtiLA1iclTDbFRZkgRQS0NqsPZphna9V+HyTEadheuPmjaJMsbzKQFOzLug==} engines: {node: '>=10'} @@ -4003,6 +4048,10 @@ packages: resolution: {integrity: sha512-nLTrUKm2UyiL7rlhapu/Zl45FwNgkZGaCpZbIHajDYgwlJCOzLSk+cIPAnsEqV955GjILJnKbdQC1nVPz+gAYQ==} engines: {node: '>= 18'} + run-applescript@7.1.0: + resolution: {integrity: sha512-DPe5pVFaAsinSaV6QjQ6gdiedWDcRCbUuiQfQa2wmWV7+xC9bGulGI8+TdRmoFkAPaBXk8CrAbnlY2ISniJ47Q==} + engines: {node: '>=18'} + run-parallel@1.2.0: resolution: {integrity: sha512-5l4VyZR86LZ/lDxZTR6jqL8AFE2S0IFLMP26AbjsLVADxHdhB/c0GUsH+y39UfCi3dzz8OlQuPmnaJOMoDHQBA==} @@ -4555,6 +4604,10 @@ packages: utf-8-validate: optional: true + wsl-utils@0.3.1: + resolution: {integrity: sha512-g/eziiSUNBSsdDJtCLB8bdYEUMj4jR7AGeUo96p/3dTafgjHhpF4RiCFPiRILwjQoDXx5MqkBr4fwWtR3Ky4Wg==} + engines: {node: '>=20'} + yaml@2.8.2: resolution: {integrity: sha512-mplynKqc1C2hTVYxd0PU2xQAc22TI1vShAYGksCCfxbn/dFwnHTNi1bvYsBTkhdUNtGIf5xNOg938rrSSYvS9A==} engines: {node: '>= 14.6'} @@ -6017,6 +6070,10 @@ snapshots: builtin-modules@5.0.0: {} + bundle-name@4.1.0: + dependencies: + run-applescript: 7.1.0 + bytes@3.1.2: {} cac@6.7.14: {} @@ -6142,12 +6199,21 @@ snapshots: deep-is@0.1.4: {} + default-browser-id@5.0.1: {} + + default-browser@5.5.0: + dependencies: + bundle-name: 4.1.0 + default-browser-id: 5.0.1 + define-data-property@1.1.4: dependencies: es-define-property: 1.0.1 es-errors: 1.3.0 gopd: 1.2.0 + define-lazy-prop@3.0.0: {} + define-properties@1.2.1: dependencies: define-data-property: 1.1.4 @@ -6924,6 +6990,8 @@ snapshots: call-bound: 1.0.4 has-tostringtag: 1.0.2 + is-docker@3.0.0: {} + is-extglob@2.1.1: {} is-finalizationregistry@1.1.1: @@ -6942,6 +7010,12 @@ snapshots: dependencies: is-extglob: 2.1.1 + is-in-ssh@1.0.0: {} + + is-inside-container@1.0.0: + dependencies: + is-docker: 3.0.0 + is-map@2.0.3: {} is-negative-zero@2.0.3: {} @@ -7000,6 +7074,10 @@ snapshots: is-windows@1.0.2: {} + is-wsl@3.1.0: + dependencies: + is-inside-container: 1.0.0 + isarray@2.0.5: {} isexe@2.0.0: {} @@ -7215,6 +7293,15 @@ snapshots: dependencies: wrappy: 1.0.2 + open@11.0.0: + dependencies: + default-browser: 5.5.0 + define-lazy-prop: 3.0.0 + is-in-ssh: 1.0.0 + is-inside-container: 1.0.0 + powershell-utils: 0.1.0 + wsl-utils: 0.3.1 + optionator@0.9.4: dependencies: deep-is: 0.1.4 @@ -7300,6 +7387,8 @@ snapshots: picocolors: 1.1.1 source-map-js: 1.2.1 + powershell-utils@0.1.0: {} + prebuild-install@7.1.3: dependencies: detect-libc: 2.1.2 @@ -7514,6 +7603,8 @@ snapshots: transitivePeerDependencies: - supports-color + run-applescript@7.1.0: {} + run-parallel@1.2.0: dependencies: queue-microtask: 1.2.3 @@ -8151,6 +8242,11 @@ snapshots: ws@8.18.3: {} + wsl-utils@0.3.1: + dependencies: + is-wsl: 3.1.0 + powershell-utils: 0.1.0 + yaml@2.8.2: {} yocto-queue@0.1.0: {}