Add path validation to prevent command injection in Process.Start calls

Co-authored-by: Kvarkas <3611964+Kvarkas@users.noreply.github.com>

mRemoteNG/Tools/ExternalTool.cs

@@ -154,15 +154,96 @@ namespace mRemoteNG.Tools
         private void SetProcessProperties(Process process, ConnectionInfo startConnectionInfo)
         {
             ExternalToolArgumentParser argParser = new(startConnectionInfo);
-            process.StartInfo.UseShellExecute = true;
-            process.StartInfo.FileName = argParser.ParseArguments(FileName);
-            var parsedArgs = argParser.ParseArguments(Arguments).Split(' ');
-            foreach (var arg in parsedArgs)
+            string parsedFileName = argParser.ParseArguments(FileName);
+            
+            // Validate the executable path to prevent command injection
+            PathValidator.ValidateExecutablePathOrThrow(parsedFileName, nameof(FileName));
+            
+            // When RunElevated is true, we must use UseShellExecute = true for the "runas" verb
+            // When false, we use UseShellExecute = false for better security with ArgumentList
+            process.StartInfo.UseShellExecute = RunElevated;
+            process.StartInfo.FileName = parsedFileName;
+            
+            if (RunElevated)
             {
-                process.StartInfo.ArgumentList.Add(arg);
+                // With UseShellExecute = true, we must use Arguments property, not ArgumentList
+                // The argument parser already handles escaping properly
+                process.StartInfo.Arguments = argParser.ParseArguments(Arguments);
+                process.StartInfo.Verb = "runas";
             }
-            if (WorkingDir != "") process.StartInfo.WorkingDirectory = argParser.ParseArguments(WorkingDir);
-            if (RunElevated) process.StartInfo.Verb = "runas";
+            else
+            {
+                // With UseShellExecute = false, use ArgumentList for better security
+                // Parse arguments using CommandLineArguments for proper splitting
+                var cmdLineArgs = new Cmdline.CommandLineArguments { EscapeForShell = false };
+                string parsedArguments = argParser.ParseArguments(Arguments);
+                
+                // Split arguments respecting quotes
+                var argumentParts = SplitCommandLineArguments(parsedArguments);
+                foreach (var arg in argumentParts)
+                {
+                    if (!string.IsNullOrWhiteSpace(arg))
+                    {
+                        process.StartInfo.ArgumentList.Add(arg);
+                    }
+                }
+            }
+            
+            if (WorkingDir != "") 
+            {
+                string parsedWorkingDir = argParser.ParseArguments(WorkingDir);
+                PathValidator.ValidatePathOrThrow(parsedWorkingDir, nameof(WorkingDir));
+                process.StartInfo.WorkingDirectory = parsedWorkingDir;
+            }
+        }
+        
+        /// <summary>
+        /// Splits command line arguments respecting quotes
+        /// </summary>
+        private static List<string> SplitCommandLineArguments(string arguments)
+        {
+            List<string> result = new();
+            if (string.IsNullOrWhiteSpace(arguments))
+                return result;
+            
+            bool inQuotes = false;
+            int startIndex = 0;
+            
+            for (int i = 0; i < arguments.Length; i++)
+            {
+                char c = arguments[i];
+                
+                if (c == '"')
+                {
+                    inQuotes = !inQuotes;
+                }
+                else if (c == ' ' && !inQuotes)
+                {
+                    if (i > startIndex)
+                    {
+                        string arg = arguments.Substring(startIndex, i - startIndex).Trim();
+                        // Remove surrounding quotes if present
+                        if (arg.StartsWith("\"") && arg.EndsWith("\"") && arg.Length > 1)
+                            arg = arg.Substring(1, arg.Length - 2);
+                        if (!string.IsNullOrWhiteSpace(arg))
+                            result.Add(arg);
+                    }
+                    startIndex = i + 1;
+                }
+            }
+            
+            // Add the last argument
+            if (startIndex < arguments.Length)
+            {
+                string arg = arguments.Substring(startIndex).Trim();
+                // Remove surrounding quotes if present
+                if (arg.StartsWith("\"") && arg.EndsWith("\"") && arg.Length > 1)
+                    arg = arg.Substring(1, arg.Length - 2);
+                if (!string.IsNullOrWhiteSpace(arg))
+                    result.Add(arg);
+            }
+            
+            return result;
         }
 
         private void StartIntegrated()

mRemoteNG/Tools/PathValidator.cs

@@ -1,5 +1,6 @@
 using System;
 using System.IO;
+using System.Linq;
 using System.Runtime.Versioning;
 
 namespace mRemoteNG.Tools
@@ -42,5 +43,61 @@ namespace mRemoteNG.Tools
             if (!IsValidPath(path))
                 throw new ArgumentException("Invalid file path: path traversal sequences are not allowed", parameterName);
         }
+
+        /// <summary>
+        /// Validates that a file path is safe to execute and doesn't contain command injection characters
+        /// </summary>
+        /// <param name="filePath">The file path to validate</param>
+        /// <returns>True if the path is safe to execute, false otherwise</returns>
+        public static bool IsValidExecutablePath(string filePath)
+        {
+            if (string.IsNullOrWhiteSpace(filePath))
+                return false;
+
+            // First check basic path validation
+            if (!IsValidPath(filePath))
+                return false;
+
+            // Check for shell metacharacters that could be used for command injection
+            // These characters are dangerous when UseShellExecute is true
+            char[] dangerousChars = ['&', '|', ';', '<', '>', '(', ')', '^', '\n', '\r'];
+            if (filePath.Any(c => dangerousChars.Contains(c)))
+                return false;
+
+            // Check for multiple consecutive quotes which could be used to break out of quoting
+            if (filePath.Contains("\"\"") || filePath.Contains("''"))
+                return false;
+
+            try
+            {
+                // Validate that the path doesn't contain invalid path characters
+                string fileName = Path.GetFileName(filePath);
+                if (string.IsNullOrWhiteSpace(fileName))
+                    return false;
+
+                // Check if path contains invalid characters
+                char[] invalidChars = Path.GetInvalidPathChars();
+                if (filePath.Any(c => invalidChars.Contains(c)))
+                    return false;
+            }
+            catch (ArgumentException)
+            {
+                return false;
+            }
+
+            return true;
+        }
+
+        /// <summary>
+        /// Validates an executable file path and throws an exception if invalid
+        /// </summary>
+        /// <param name="filePath">The file path to validate</param>
+        /// <param name="parameterName">The name of the parameter being validated</param>
+        /// <exception cref="ArgumentException">Thrown when the path is not safe to execute</exception>
+        public static void ValidateExecutablePathOrThrow(string filePath, string parameterName = "filePath")
+        {
+            if (!IsValidExecutablePath(filePath))
+                throw new ArgumentException("Invalid executable path: path contains potentially dangerous characters or sequences", parameterName);
+        }
     }
 }

mRemoteNG/Tools/ProcessController.cs

@@ -16,6 +16,9 @@ namespace mRemoteNG.Tools
 
         public bool Start(string fileName, CommandLineArguments arguments = null)
         {
+            // Validate the executable path to prevent command injection
+            PathValidator.ValidateExecutablePathOrThrow(fileName, nameof(fileName));
+            
             Process.StartInfo.UseShellExecute = false;
             Process.StartInfo.FileName = fileName;
             if (arguments != null)