From c9b77b6616bf05ad612077b0200f48c64ba663fb Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 22 Oct 2025 08:17:58 +0000 Subject: [PATCH] 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 | 131 +++++++++++++++++ mRemoteNGTests/Tools/PathValidatorTests.cs | 157 +++++++++++++++++++++ 2 files changed, 288 insertions(+) diff --git a/mRemoteNGTests/Tools/ExternalToolTests.cs b/mRemoteNGTests/Tools/ExternalToolTests.cs index 7334c0243..2ae5fff76 100644 --- a/mRemoteNGTests/Tools/ExternalToolTests.cs +++ b/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(() => + setProcessPropertiesMethod?.Invoke(externalTool, new object[] { process, connectionInfo })); + Assert.That(ex.InnerException, Is.TypeOf()); + } + + [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); + } } } diff --git a/mRemoteNGTests/Tools/PathValidatorTests.cs b/mRemoteNGTests/Tools/PathValidatorTests.cs index 7874a268f..8afdf6dff 100644 --- a/mRemoteNGTests/Tools/PathValidatorTests.cs +++ b/mRemoteNGTests/Tools/PathValidatorTests.cs @@ -88,4 +88,161 @@ public class PathValidatorTests var exception = Assert.Throws(() => 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(() => PathValidator.ValidateExecutablePathOrThrow(maliciousPath)); + Assert.That(exception.Message, Does.Contain("dangerous characters")); + } + + [Test] + public void ValidateExecutablePathOrThrow_PathWithTraversal_ThrowsArgumentException() + { + string maliciousPath = @"..\notepad.exe"; + Assert.Throws(() => PathValidator.ValidateExecutablePathOrThrow(maliciousPath)); + } + + [Test] + public void ValidateExecutablePathOrThrow_WithCustomParameterName_IncludesParameterName() + { + string maliciousPath = @"notepad.exe & calc.exe"; + var exception = Assert.Throws(() => PathValidator.ValidateExecutablePathOrThrow(maliciousPath, "executablePath")); + Assert.That(exception.ParamName, Is.EqualTo("executablePath")); + } + + #endregion }