Add comprehensive security tests for path validation and command injection prevention

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

mRemoteNGTests/Tools/ExternalToolTests.cs

@@ -111,5 +111,136 @@ namespace mRemoteNGTests.Tools
             Assert.That(arguments, Does.Contain("TestPass"));
             Assert.That(arguments, Does.Contain("123"));
         }
+
+        [Test]
+        public void ArgumentsWithSpaces_AreParsedCorrectly()
+        {
+            // Arrange
+            var connectionInfo = new ConnectionInfo
+            {
+                Hostname = "test host",
+                Username = "user name"
+            };
+
+            var externalTool = new ExternalTool
+            {
+                DisplayName = "Test Tool",
+                FileName = "app.exe",
+                Arguments = "--host \"%HOSTNAME%\" --user \"%USERNAME%\""
+            };
+
+            // Act
+            var process = new Process();
+            var setProcessPropertiesMethod = typeof(ExternalTool).GetMethod(
+                "SetProcessProperties",
+                BindingFlags.NonPublic | BindingFlags.Instance
+            );
+            setProcessPropertiesMethod?.Invoke(externalTool, new object[] { process, connectionInfo });
+
+            // Assert - When not elevated, arguments should be in ArgumentList
+            if (!externalTool.RunElevated)
+            {
+                Assert.That(process.StartInfo.ArgumentList.Count, Is.GreaterThan(0));
+                // Arguments with spaces should be preserved in ArgumentList
+                Assert.That(process.StartInfo.ArgumentList, Does.Contain("test host").Or.Contain("--host"));
+            }
+        }
+
+        [Test]
+        public void ValidExecutablePath_DoesNotThrow()
+        {
+            // Arrange
+            var connectionInfo = new ConnectionInfo();
+            var externalTool = new ExternalTool
+            {
+                DisplayName = "Test Tool",
+                FileName = "notepad.exe",
+                Arguments = ""
+            };
+
+            // Act & Assert
+            var process = new Process();
+            var setProcessPropertiesMethod = typeof(ExternalTool).GetMethod(
+                "SetProcessProperties",
+                BindingFlags.NonPublic | BindingFlags.Instance
+            );
+            
+            Assert.DoesNotThrow(() => setProcessPropertiesMethod?.Invoke(externalTool, new object[] { process, connectionInfo }));
+        }
+
+        [Test]
+        public void InvalidExecutablePath_ThrowsArgumentException()
+        {
+            // Arrange
+            var connectionInfo = new ConnectionInfo();
+            var externalTool = new ExternalTool
+            {
+                DisplayName = "Test Tool",
+                FileName = "notepad.exe & calc.exe", // Command injection attempt
+                Arguments = ""
+            };
+
+            // Act & Assert
+            var process = new Process();
+            var setProcessPropertiesMethod = typeof(ExternalTool).GetMethod(
+                "SetProcessProperties",
+                BindingFlags.NonPublic | BindingFlags.Instance
+            );
+
+            var ex = Assert.Throws<TargetInvocationException>(() => 
+                setProcessPropertiesMethod?.Invoke(externalTool, new object[] { process, connectionInfo }));
+            Assert.That(ex.InnerException, Is.TypeOf<ArgumentException>());
+        }
+
+        [Test]
+        public void RunElevated_UsesShellExecuteTrue()
+        {
+            // Arrange
+            var connectionInfo = new ConnectionInfo();
+            var externalTool = new ExternalTool
+            {
+                DisplayName = "Test Tool",
+                FileName = "notepad.exe",
+                Arguments = "--test",
+                RunElevated = true
+            };
+
+            // Act
+            var process = new Process();
+            var setProcessPropertiesMethod = typeof(ExternalTool).GetMethod(
+                "SetProcessProperties",
+                BindingFlags.NonPublic | BindingFlags.Instance
+            );
+            setProcessPropertiesMethod?.Invoke(externalTool, new object[] { process, connectionInfo });
+
+            // Assert
+            Assert.That(process.StartInfo.UseShellExecute, Is.True);
+            Assert.That(process.StartInfo.Verb, Is.EqualTo("runas"));
+        }
+
+        [Test]
+        public void RunNotElevated_UsesShellExecuteFalse()
+        {
+            // Arrange
+            var connectionInfo = new ConnectionInfo();
+            var externalTool = new ExternalTool
+            {
+                DisplayName = "Test Tool",
+                FileName = "notepad.exe",
+                Arguments = "--test",
+                RunElevated = false
+            };
+
+            // Act
+            var process = new Process();
+            var setProcessPropertiesMethod = typeof(ExternalTool).GetMethod(
+                "SetProcessProperties",
+                BindingFlags.NonPublic | BindingFlags.Instance
+            );
+            setProcessPropertiesMethod?.Invoke(externalTool, new object[] { process, connectionInfo });
+
+            // Assert
+            Assert.That(process.StartInfo.UseShellExecute, Is.False);
+        }
     }
 }

mRemoteNGTests/Tools/PathValidatorTests.cs

@@ -88,4 +88,161 @@ public class PathValidatorTests
         var exception = Assert.Throws<ArgumentException>(() => PathValidator.ValidatePathOrThrow(maliciousPath, "customParam"));
         Assert.That(exception.ParamName, Is.EqualTo("customParam"));
     }
+
+    #region IsValidExecutablePath Tests
+
+    [Test]
+    public void IsValidExecutablePath_ValidWindowsPath_ReturnsTrue()
+    {
+        string validPath = @"C:\Windows\System32\notepad.exe";
+        Assert.That(PathValidator.IsValidExecutablePath(validPath), Is.True);
+    }
+
+    [Test]
+    public void IsValidExecutablePath_ValidRelativePath_ReturnsTrue()
+    {
+        string validPath = @"app.exe";
+        Assert.That(PathValidator.IsValidExecutablePath(validPath), Is.True);
+    }
+
+    [Test]
+    public void IsValidExecutablePath_PathWithSpaces_ReturnsTrue()
+    {
+        string validPath = @"C:\Program Files\App\app.exe";
+        Assert.That(PathValidator.IsValidExecutablePath(validPath), Is.True);
+    }
+
+    [Test]
+    public void IsValidExecutablePath_PathWithCommandInjectionAmpersand_ReturnsFalse()
+    {
+        string maliciousPath = @"notepad.exe & calc.exe";
+        Assert.That(PathValidator.IsValidExecutablePath(maliciousPath), Is.False);
+    }
+
+    [Test]
+    public void IsValidExecutablePath_PathWithCommandInjectionPipe_ReturnsFalse()
+    {
+        string maliciousPath = @"notepad.exe | calc.exe";
+        Assert.That(PathValidator.IsValidExecutablePath(maliciousPath), Is.False);
+    }
+
+    [Test]
+    public void IsValidExecutablePath_PathWithCommandInjectionSemicolon_ReturnsFalse()
+    {
+        string maliciousPath = @"notepad.exe; calc.exe";
+        Assert.That(PathValidator.IsValidExecutablePath(maliciousPath), Is.False);
+    }
+
+    [Test]
+    public void IsValidExecutablePath_PathWithRedirection_ReturnsFalse()
+    {
+        string maliciousPath = @"notepad.exe > output.txt";
+        Assert.That(PathValidator.IsValidExecutablePath(maliciousPath), Is.False);
+    }
+
+    [Test]
+    public void IsValidExecutablePath_PathWithInputRedirection_ReturnsFalse()
+    {
+        string maliciousPath = @"notepad.exe < input.txt";
+        Assert.That(PathValidator.IsValidExecutablePath(maliciousPath), Is.False);
+    }
+
+    [Test]
+    public void IsValidExecutablePath_PathWithParentheses_ReturnsFalse()
+    {
+        string maliciousPath = @"notepad.exe (calc.exe)";
+        Assert.That(PathValidator.IsValidExecutablePath(maliciousPath), Is.False);
+    }
+
+    [Test]
+    public void IsValidExecutablePath_PathWithCaret_ReturnsFalse()
+    {
+        string maliciousPath = @"notepad.exe^calc.exe";
+        Assert.That(PathValidator.IsValidExecutablePath(maliciousPath), Is.False);
+    }
+
+    [Test]
+    public void IsValidExecutablePath_PathWithNewline_ReturnsFalse()
+    {
+        string maliciousPath = "notepad.exe\ncalc.exe";
+        Assert.That(PathValidator.IsValidExecutablePath(maliciousPath), Is.False);
+    }
+
+    [Test]
+    public void IsValidExecutablePath_PathWithCarriageReturn_ReturnsFalse()
+    {
+        string maliciousPath = "notepad.exe\rcalc.exe";
+        Assert.That(PathValidator.IsValidExecutablePath(maliciousPath), Is.False);
+    }
+
+    [Test]
+    public void IsValidExecutablePath_PathWithMultipleQuotes_ReturnsFalse()
+    {
+        string maliciousPath = @"notepad.exe """"";
+        Assert.That(PathValidator.IsValidExecutablePath(maliciousPath), Is.False);
+    }
+
+    [Test]
+    public void IsValidExecutablePath_PathWithMultipleSingleQuotes_ReturnsFalse()
+    {
+        string maliciousPath = @"notepad.exe ''";
+        Assert.That(PathValidator.IsValidExecutablePath(maliciousPath), Is.False);
+    }
+
+    [Test]
+    public void IsValidExecutablePath_EmptyPath_ReturnsFalse()
+    {
+        Assert.That(PathValidator.IsValidExecutablePath(""), Is.False);
+    }
+
+    [Test]
+    public void IsValidExecutablePath_NullPath_ReturnsFalse()
+    {
+        Assert.That(PathValidator.IsValidExecutablePath(null), Is.False);
+    }
+
+    [Test]
+    public void IsValidExecutablePath_WhitespacePath_ReturnsFalse()
+    {
+        Assert.That(PathValidator.IsValidExecutablePath("   "), Is.False);
+    }
+
+    [Test]
+    public void IsValidExecutablePath_PathWithTraversal_ReturnsFalse()
+    {
+        string maliciousPath = @"..\notepad.exe";
+        Assert.That(PathValidator.IsValidExecutablePath(maliciousPath), Is.False);
+    }
+
+    [Test]
+    public void ValidateExecutablePathOrThrow_ValidPath_DoesNotThrow()
+    {
+        string validPath = @"C:\Windows\System32\notepad.exe";
+        Assert.DoesNotThrow(() => PathValidator.ValidateExecutablePathOrThrow(validPath));
+    }
+
+    [Test]
+    public void ValidateExecutablePathOrThrow_PathWithCommandInjection_ThrowsArgumentException()
+    {
+        string maliciousPath = @"notepad.exe & calc.exe";
+        var exception = Assert.Throws<ArgumentException>(() => PathValidator.ValidateExecutablePathOrThrow(maliciousPath));
+        Assert.That(exception.Message, Does.Contain("dangerous characters"));
+    }
+
+    [Test]
+    public void ValidateExecutablePathOrThrow_PathWithTraversal_ThrowsArgumentException()
+    {
+        string maliciousPath = @"..\notepad.exe";
+        Assert.Throws<ArgumentException>(() => PathValidator.ValidateExecutablePathOrThrow(maliciousPath));
+    }
+
+    [Test]
+    public void ValidateExecutablePathOrThrow_WithCustomParameterName_IncludesParameterName()
+    {
+        string maliciousPath = @"notepad.exe & calc.exe";
+        var exception = Assert.Throws<ArgumentException>(() => PathValidator.ValidateExecutablePathOrThrow(maliciousPath, "executablePath"));
+        Assert.That(exception.ParamName, Is.EqualTo("executablePath"));
+    }
+
+    #endregion
 }