From f52790cec0f266c973f9a1567738babf574ce1b4 Mon Sep 17 00:00:00 2001 From: Ryan Malloy Date: Thu, 5 Feb 2026 10:39:52 -0700 Subject: [PATCH] security: fix shell injection and add subprocess timeout - Replace create_subprocess_shell with create_subprocess_exec in _try_install_dotnet_sdk() to prevent shell injection - Add install_commands list to _detect_platform() returning safe argument lists for each platform - Add 5-minute timeout to ilspy_wrapper._run_command() to prevent hanging on malicious/corrupted assemblies --- src/mcilspy/ilspy_wrapper.py | 13 ++++- src/mcilspy/server.py | 101 +++++++++++++++++++++++++---------- 2 files changed, 85 insertions(+), 29 deletions(-) diff --git a/src/mcilspy/ilspy_wrapper.py b/src/mcilspy/ilspy_wrapper.py index e407176..d29d224 100644 --- a/src/mcilspy/ilspy_wrapper.py +++ b/src/mcilspy/ilspy_wrapper.py @@ -71,7 +71,18 @@ class ILSpyWrapper: ) input_bytes = input_data.encode("utf-8") if input_data else None - stdout_bytes, stderr_bytes = await process.communicate(input=input_bytes) + + # Timeout after 5 minutes to prevent hanging on malicious/corrupted assemblies + try: + stdout_bytes, stderr_bytes = await asyncio.wait_for( + process.communicate(input=input_bytes), + timeout=300.0 # 5 minutes + ) + except asyncio.TimeoutError: + logger.warning(f"Command timed out after 5 minutes, killing process") + process.kill() + await process.wait() # Ensure process is cleaned up + return -1, "", "Command timed out after 5 minutes. The assembly may be corrupted or too complex." stdout = stdout_bytes.decode("utf-8", errors="replace") if stdout_bytes else "" stderr = stderr_bytes.decode("utf-8", errors="replace") if stderr_bytes else "" diff --git a/src/mcilspy/server.py b/src/mcilspy/server.py index 45017f5..d477f10 100644 --- a/src/mcilspy/server.py +++ b/src/mcilspy/server.py @@ -81,13 +81,23 @@ async def _check_dotnet_tools() -> dict: def _detect_platform() -> dict: - """Detect the platform and recommend the appropriate .NET SDK install command.""" + """Detect the platform and recommend the appropriate .NET SDK install command. + + Returns a dict with: + - system: OS name (linux, darwin, windows) + - distro: Distribution name if detected + - package_manager: Package manager name + - install_command: Human-readable command string (for display) + - install_commands: List of command arg lists for safe execution (no shell) + - needs_sudo: Whether installation requires elevated privileges + """ system = platform.system().lower() result = { "system": system, "distro": None, "package_manager": None, "install_command": None, + "install_commands": None, # List of [arg, list, ...] for subprocess_exec "needs_sudo": True, } @@ -99,39 +109,63 @@ def _detect_platform() -> dict: if "arch" in os_release or "manjaro" in os_release or "endeavour" in os_release: result["distro"] = "arch" result["package_manager"] = "pacman" - result["install_command"] = "sudo pacman -S dotnet-sdk" + result["install_command"] = "sudo pacman -S --noconfirm dotnet-sdk" + result["install_commands"] = [ + ["sudo", "pacman", "-S", "--noconfirm", "dotnet-sdk"] + ] elif "ubuntu" in os_release or "debian" in os_release or "mint" in os_release: result["distro"] = "debian" result["package_manager"] = "apt" result["install_command"] = "sudo apt update && sudo apt install -y dotnet-sdk-8.0" + result["install_commands"] = [ + ["sudo", "apt", "update"], + ["sudo", "apt", "install", "-y", "dotnet-sdk-8.0"], + ] elif "fedora" in os_release or "rhel" in os_release or "centos" in os_release: result["distro"] = "fedora" result["package_manager"] = "dnf" result["install_command"] = "sudo dnf install -y dotnet-sdk-8.0" + result["install_commands"] = [ + ["sudo", "dnf", "install", "-y", "dotnet-sdk-8.0"] + ] elif "opensuse" in os_release or "suse" in os_release: result["distro"] = "suse" result["package_manager"] = "zypper" result["install_command"] = "sudo zypper install -y dotnet-sdk-8.0" + result["install_commands"] = [ + ["sudo", "zypper", "install", "-y", "dotnet-sdk-8.0"] + ] except FileNotFoundError: pass # Fallback: check for common package managers - if result["install_command"] is None: + if result["install_commands"] is None: if shutil.which("pacman"): result["package_manager"] = "pacman" - result["install_command"] = "sudo pacman -S dotnet-sdk" + result["install_command"] = "sudo pacman -S --noconfirm dotnet-sdk" + result["install_commands"] = [ + ["sudo", "pacman", "-S", "--noconfirm", "dotnet-sdk"] + ] elif shutil.which("apt"): result["package_manager"] = "apt" result["install_command"] = "sudo apt update && sudo apt install -y dotnet-sdk-8.0" + result["install_commands"] = [ + ["sudo", "apt", "update"], + ["sudo", "apt", "install", "-y", "dotnet-sdk-8.0"], + ] elif shutil.which("dnf"): result["package_manager"] = "dnf" result["install_command"] = "sudo dnf install -y dotnet-sdk-8.0" + result["install_commands"] = [ + ["sudo", "dnf", "install", "-y", "dotnet-sdk-8.0"] + ] elif system == "darwin": result["distro"] = "macos" if shutil.which("brew"): result["package_manager"] = "homebrew" result["install_command"] = "brew install dotnet-sdk" + result["install_commands"] = [["brew", "install", "dotnet-sdk"]] result["needs_sudo"] = False else: result["install_command"] = ( @@ -143,10 +177,14 @@ def _detect_platform() -> dict: if shutil.which("winget"): result["package_manager"] = "winget" result["install_command"] = "winget install Microsoft.DotNet.SDK.8" + result["install_commands"] = [ + ["winget", "install", "Microsoft.DotNet.SDK.8", "--accept-source-agreements"] + ] result["needs_sudo"] = False elif shutil.which("choco"): result["package_manager"] = "chocolatey" result["install_command"] = "choco install dotnet-sdk -y" + result["install_commands"] = [["choco", "install", "dotnet-sdk", "-y"]] else: result["install_command"] = "Download from https://dotnet.microsoft.com/download" result["needs_sudo"] = False @@ -163,44 +201,51 @@ async def _try_install_dotnet_sdk(ctx: Context | None = None) -> tuple[bool, str """Attempt to install the .NET SDK using the detected package manager. Returns (success, message) tuple. + + Security: Uses create_subprocess_exec with explicit argument lists + to avoid shell injection vulnerabilities. """ platform_info = _detect_platform() - cmd = platform_info.get("install_command") + commands = platform_info.get("install_commands") + display_cmd = platform_info.get("install_command", "") - if not cmd or "Download from" in cmd: + if not commands: return False, ( f"Cannot auto-install .NET SDK on {platform_info['system']}.\n\n" - f"Please install manually: {cmd}" + f"Please install manually: {display_cmd}" ) - if platform_info.get("needs_sudo") and os.geteuid() != 0: - # We need sudo but aren't root - try anyway, it might prompt or fail gracefully - pass - if ctx: await ctx.info(f"Installing .NET SDK via {platform_info['package_manager']}...") + all_output = [] try: - # Use shell=True to handle complex commands with && and sudo - proc = await asyncio.create_subprocess_shell( - cmd, - stdout=asyncio.subprocess.PIPE, - stderr=asyncio.subprocess.PIPE, - ) - stdout, stderr = await proc.communicate() - output = stdout.decode() + stderr.decode() + # Execute each command in sequence (e.g., apt update, then apt install) + for cmd_args in commands: + if ctx: + await ctx.info(f"Running: {' '.join(cmd_args)}") - if proc.returncode == 0: - return True, f"✅ .NET SDK installed successfully via {platform_info['package_manager']}!" - else: - return False, ( - f"❌ Installation failed (exit code {proc.returncode}).\n\n" - f"Command: `{cmd}`\n\n" - f"Output:\n```\n{output[-1000:]}\n```\n\n" - "Try running the command manually with sudo if needed." + proc = await asyncio.create_subprocess_exec( + *cmd_args, + stdout=asyncio.subprocess.PIPE, + stderr=asyncio.subprocess.PIPE, ) + stdout, stderr = await proc.communicate() + output = stdout.decode() + stderr.decode() + all_output.append(f"$ {' '.join(cmd_args)}\n{output}") + + if proc.returncode != 0: + return False, ( + f"❌ Installation failed (exit code {proc.returncode}).\n\n" + f"Command: `{' '.join(cmd_args)}`\n\n" + f"Output:\n```\n{output[-1000:]}\n```\n\n" + "Try running the command manually with sudo if needed." + ) + + return True, f"✅ .NET SDK installed successfully via {platform_info['package_manager']}!" + except Exception as e: - return False, f"❌ Failed to run install command: {e}\n\nTry manually: `{cmd}`" + return False, f"❌ Failed to run install command: {e}\n\nTry manually: `{display_cmd}`" @mcp.tool()