From e8dac2048faca2f6db266ecf2778f61005384431 Mon Sep 17 00:00:00 2001 From: Henry Heng Date: Wed, 30 Jul 2025 16:44:20 +0100 Subject: [PATCH] Bugfix/Custom MCP Security (#4963) * - Implemented a validation function to check for banned commands and dangerous patterns. - Added checks for potential shell injection attempts in command and arguments. - Security validation is conditionally enabled based on environment variable CUSTOM_MCP_SECURITY_CHECK. * Enhance security by implementing command and argument validation in SupergatewayMCP. Added checks for banned commands, dangerous patterns, and potential shell injection attempts. Security validation is conditionally enabled based on the CUSTOM_MCP_SECURITY_CHECK environment variable. * add validateMCPServerSecurity --- .../nodes/tools/MCP/CustomMCP/CustomMCP.ts | 6 +- .../tools/MCP/Supergateway/SupergatewayMCP.ts | 35 +- packages/components/nodes/tools/MCP/core.ts | 397 ++++++++++++++++++ 3 files changed, 423 insertions(+), 15 deletions(-) diff --git a/packages/components/nodes/tools/MCP/CustomMCP/CustomMCP.ts b/packages/components/nodes/tools/MCP/CustomMCP/CustomMCP.ts index b10ece77..ff39a21f 100644 --- a/packages/components/nodes/tools/MCP/CustomMCP/CustomMCP.ts +++ b/packages/components/nodes/tools/MCP/CustomMCP/CustomMCP.ts @@ -1,6 +1,6 @@ import { Tool } from '@langchain/core/tools' import { ICommonObject, IDatabaseEntity, INode, INodeData, INodeOptionsValue, INodeParams } from '../../../../src/Interface' -import { MCPToolkit } from '../core' +import { MCPToolkit, validateMCPServerSecurity } from '../core' import { getVars, prepareSandboxVars } from '../../../../src/utils' import { DataSource } from 'typeorm' import hash from 'object-hash' @@ -169,6 +169,10 @@ class Custom_MCP implements INode { serverParams = JSON.parse(serverParamsString) } + if (process.env.CUSTOM_MCP_SECURITY_CHECK === 'true') { + validateMCPServerSecurity(serverParams) + } + // Compatible with stdio and SSE let toolkit: MCPToolkit if (serverParams?.command === undefined) { diff --git a/packages/components/nodes/tools/MCP/Supergateway/SupergatewayMCP.ts b/packages/components/nodes/tools/MCP/Supergateway/SupergatewayMCP.ts index b8e1d628..347a3577 100644 --- a/packages/components/nodes/tools/MCP/Supergateway/SupergatewayMCP.ts +++ b/packages/components/nodes/tools/MCP/Supergateway/SupergatewayMCP.ts @@ -1,7 +1,7 @@ import { Tool } from '@langchain/core/tools' import { ICommonObject, INode, INodeData, INodeOptionsValue, INodeParams } from '../../../../src/Interface' import { getNodeModulesPackagePath } from '../../../../src/utils' -import { MCPToolkit } from '../core' +import { MCPToolkit, validateMCPServerSecurity } from '../core' class Supergateway_MCP implements INode { label: string @@ -90,21 +90,28 @@ class Supergateway_MCP implements INode { const _args = nodeData.inputs?.arguments as string const packagePath = getNodeModulesPackagePath('supergateway/dist/index.js') + const processedArgs = _args + .trim() + .split(/\s+/) + .map((arg) => { + // Remove surrounding double or single quotes if they exist + if ((arg.startsWith('"') && arg.endsWith('"')) || (arg.startsWith("'") && arg.endsWith("'"))) { + return arg.slice(1, -1) + } + return arg + }) + const serverParams = { command: 'node', - args: [ - packagePath, - ..._args - .trim() - .split(/\s+/) - .map((arg) => { - // Remove surrounding double or single quotes if they exist - if ((arg.startsWith('"') && arg.endsWith('"')) || (arg.startsWith("'") && arg.endsWith("'"))) { - return arg.slice(1, -1) - } - return arg - }) - ] + args: [packagePath, ...processedArgs] + } + + if (process.env.CUSTOM_MCP_SECURITY_CHECK === 'true') { + try { + validateMCPServerSecurity(serverParams) + } catch (error) { + throw new Error(`Security validation failed: ${error.message}`) + } } const toolkit = new MCPToolkit(serverParams, 'stdio') diff --git a/packages/components/nodes/tools/MCP/core.ts b/packages/components/nodes/tools/MCP/core.ts index 6c6f49f2..be9e65f5 100644 --- a/packages/components/nodes/tools/MCP/core.ts +++ b/packages/components/nodes/tools/MCP/core.ts @@ -173,3 +173,400 @@ function createSchemaModel( return z.object(schemaProperties) } + +/** + * TODO: To be removed and only allow Remote MCP for Cloud + * Validates MCP server configuration to prevent execution of dangerous commands + */ +export function validateMCPServerSecurity(serverParams: any): void { + // Comprehensive list of dangerous commands that could compromise system security + const dangerousCommands = [ + // Shell interpreters and command processors + 'sh', + 'bash', + 'zsh', + 'fish', + 'csh', + 'tcsh', + 'ksh', + 'ash', + 'dash', + 'cmd', + 'command', + 'powershell', + 'pwsh', + 'cmd.exe', + 'powershell.exe', + 'wsl', + 'wsl.exe', + 'ubuntu', + 'debian', + + // File operations that could read/write sensitive files + 'cat', + 'more', + 'less', + 'head', + 'tail', + 'tee', + 'cp', + 'mv', + 'rm', + 'del', + 'copy', + 'move', + 'type', + 'ren', + 'rename', + 'ln', + 'link', + 'unlink', + 'touch', + 'mkdir', + 'rmdir', + 'rd', + 'md', + 'makedir', + + // Directory operations and file system navigation + 'ls', + 'dir', + 'pwd', + 'cd', + 'find', + 'locate', + 'tree', + 'du', + 'df', + 'pushd', + 'popd', + 'dirs', + 'whereis', + 'which', + 'where', + 'stat', + + // Network operations that could exfiltrate data or download malicious content + 'curl', + 'wget', + 'nc', + 'netcat', + 'ping', + 'nslookup', + 'dig', + 'host', + 'telnet', + 'ssh', + 'scp', + 'rsync', + 'ftp', + 'sftp', + 'icat', + 'socat', + + // System operations and process management + 'ps', + 'top', + 'htop', + 'kill', + 'killall', + 'pkill', + 'pgrep', + 'jobs', + 'systemctl', + 'service', + 'chkconfig', + 'update-rc.d', + 'systemd', + 'crontab', + 'at', + 'batch', + 'nohup', + 'screen', + 'tmux', + + // Archive operations that could be used for data exfiltration + 'tar', + 'zip', + 'unzip', + 'gzip', + 'gunzip', + 'bzip2', + 'bunzip2', + 'xz', + 'unxz', + '7z', + 'rar', + 'unrar', + 'compress', + 'uncompress', + 'cpio', + 'ar', + + // Text processing tools that could read sensitive files + 'grep', + 'awk', + 'sed', + 'sort', + 'uniq', + 'cut', + 'tr', + 'wc', + 'diff', + 'patch', + 'strings', + 'hexdump', + 'od', + 'xxd', + 'base64', + 'base32', + + // File editors that could modify system files + 'vi', + 'vim', + 'nano', + 'emacs', + 'gedit', + 'notepad', + 'notepad.exe', + 'pico', + 'joe', + 'micro', + 'code', + 'subl', + 'atom', + + // Process execution and evaluation commands + 'exec', + 'eval', + 'system', + 'spawn', + 'fork', + 'clone', + 'source', + 'sudo', + 'su', + 'runuser', + 'doas', + 'pfexec', + + // Package managers that could install malicious software + 'npm', + 'yarn', + 'pnpm', + 'pip', + 'pip3', + 'apt', + 'apt-get', + 'yum', + 'dnf', + 'pacman', + 'brew', + 'choco', + 'winget', + 'scoop', + 'snap', + 'flatpak', + + // Compilers and interpreters that could execute arbitrary code + 'python', + 'python3', + 'nodejs', + 'java', + 'javac', + 'gcc', + 'g++', + 'clang', + 'clang++', + 'ruby', + 'perl', + 'php', + 'go', + 'rust', + 'cargo', + 'dotnet', + 'mono', + 'scala', + 'kotlin', + 'swift', + + // Database clients that could access sensitive data + 'mysql', + 'psql', + 'mongo', + 'mongosh', + 'redis-cli', + 'sqlite3', + 'sqlcmd', + 'isql', + 'osql', + 'bcp', + + // Development and deployment tools + 'git', + 'docker', + 'podman', + 'kubectl', + 'helm', + 'terraform', + 'ansible', + 'vagrant', + 'chef', + 'puppet', + 'saltstack', + 'make', + 'cmake', + + // System information and configuration tools + 'uname', + 'whoami', + 'id', + 'groups', + 'env', + 'set', + 'printenv', + 'export', + 'mount', + 'umount', + 'lsblk', + 'fdisk', + 'parted', + 'lsmod', + 'modprobe', + + // File permissions and ownership commands + 'chmod', + 'chown', + 'chgrp', + 'umask', + 'setfacl', + 'getfacl', + 'lsattr', + 'chattr', + + // Network configuration and monitoring + 'ifconfig', + 'ip', + 'route', + 'netstat', + 'ss', + 'lsof', + 'tcpdump', + 'wireshark', + 'iptables', + 'ufw', + 'firewall-cmd', + + // Boot and system control + 'init', + 'telinit', + 'shutdown', + 'reboot', + 'halt', + 'poweroff', + + // Hardware and kernel interaction + 'dmesg', + 'lspci', + 'lsusb', + 'dmidecode', + 'hdparm', + 'smartctl' + ] + + /** + * Checks a string for dangerous commands and patterns + * @param str - The string to check + * @param context - Context information for better error messages + */ + function checkString(str: string, context: string = ''): void { + if (typeof str !== 'string') return + + const lowerStr = str.toLowerCase().trim() + const contextPrefix = context ? `${context}: ` : '' + + for (const cmd of dangerousCommands) { + const cmdLower = cmd.toLowerCase() + // Escape special regex characters in command name + const escapedCmd = cmdLower.replace(/[.*+?^${}()|[\]\\]/g, '\\$&') + + if ( + lowerStr === cmdLower || // 1. Exact match: "cat" === "cat" + lowerStr.startsWith(cmdLower + ' ') || // 2. Command with space args: "cat /etc/passwd" + lowerStr.startsWith(cmdLower + '\t') || // 3. Command with tab args: "cat\t/etc/passwd" + lowerStr.endsWith('/' + cmdLower) || // 4. Unix absolute path: "/bin/sh" + lowerStr.includes('\\' + cmdLower + '.exe') || // 5. Windows executable: "C:\\Windows\\cmd.exe" + lowerStr.includes('/' + cmdLower + ' ') || // 6. Unix path with args: "/bin/sh -c" + lowerStr.includes('\\' + cmdLower + ' ') || // 7. Windows path with args: "C:\\bin\\sh.exe -c" + new RegExp(`\\b${escapedCmd}\\b`).test(lowerStr) + ) { + // 8. Word boundary match: catches embedded commands + throw new Error(`${contextPrefix}Dangerous command detected: "${cmd}" in "${str}"`) + } + } + + // 2. Check for null bytes (binary content or encoding attacks) + if (str.includes('\0')) { + throw new Error(`${contextPrefix}Null byte detected in string: "${str}"`) + } + } + + /** + * Recursively validates an object for dangerous content + * This function traverses the entire object tree to ensure no malicious content is hidden + * @param obj - The object to validate (can be string, array, object, or primitive) + * @param path - The current path in the object (for error reporting and debugging) + */ + function validateObject(obj: any, path: string = ''): void { + // Skip null/undefined values + if (obj === null || obj === undefined) return + + if (typeof obj === 'string') { + // Validate string content for dangerous commands and patterns + checkString(obj, path) + } else if (Array.isArray(obj)) { + // Recursively validate each array element + obj.forEach((item, index) => { + validateObject(item, `${path}[${index}]`) + }) + } else if (typeof obj === 'object') { + // Recursively validate each object property + for (const [key, value] of Object.entries(obj)) { + const currentPath = path ? `${path}.${key}` : key + // Validate only the property value + validateObject(value, currentPath) + } + } + } + + validateObject(serverParams, 'serverParams') + + if (serverParams.command) { + const cmd = serverParams.command.toLowerCase() + if (cmd.includes('sh') || cmd.includes('cmd') || cmd.includes('powershell') || cmd.includes('eval')) { + throw new Error(`Command field contains dangerous interpreter: "${serverParams.command}"`) + } + } + + if (serverParams.env) { + for (const [key, value] of Object.entries(serverParams.env)) { + if (typeof value === 'string' && (value.includes('$(') || value.includes('`'))) { + throw new Error(`Environment variable "${key}" contains command substitution: "${value}"`) + } + } + } + + if (serverParams.cwd) { + checkString(serverParams.cwd, 'cwd') + const cwd = serverParams.cwd.toLowerCase() + if ( + cwd.startsWith('/bin') || + cwd.startsWith('/sbin') || + cwd.startsWith('/etc') || + cwd.includes('system32') || + cwd.includes('program files') + ) { + throw new Error(`Working directory points to sensitive system location: "${serverParams.cwd}"`) + } + } +}