Learning/mRemoteNG
1
0
Fork 0
mirror of https://github.com/mRemoteNG/mRemoteNG.git synced 2026-02-17 22:11:48 +08:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #3038 from mRemoteNG/copilot/fix-command-injection-vulnerability

Browse Source 
Fix command injection vulnerabilities in Process.Start calls
This commit is contained in:
Dimitrij
2026-02-10 21:18:59 +00:00
committed by GitHub
parent 333588e101 91c7df22b2
commit 5efcc653eb
8 changed files with 389 additions and 41 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
mRemoteNG/App/Shutdown.cs
View File

@@ -121,7 +121,11 @@ namespace mRemoteNG.App
private static void RunUpdateFile()
{
if (UpdatePending)
{
// Validate the update file path to prevent command injection
Tools.PathValidator.ValidateExecutablePathOrThrow(_updateFilePath, nameof(_updateFilePath));
Process.Start(new ProcessStartInfo(_updateFilePath) { UseShellExecute = true });
}
}
}
}

65
mRemoteNG/Connection/Protocol/AnyDesk/ProtocolAnyDesk.cs
View File

@@ -211,7 +211,17 @@ namespace mRemoteNG.Connection.Protocol.AnyDesk
// Username field is optional and not used in the CLI (reserved for future use)
// Password field is piped via stdin when --with-password flag is used
string anydeskId = _connectionInfo.Hostname.Trim();
string arguments = $"{anydeskId}";
// Validate AnyDesk ID to prevent command injection
// AnyDesk IDs are numeric or alphanumeric with @ and - characters for aliases
if (!IsValidAnydeskId(anydeskId))
{
Runtime.MessageCollector?.AddMessage(MessageClass.ErrorMsg,
"Invalid AnyDesk ID format. Only alphanumeric characters, @, -, _, and . are allowed.", true);
return false;
}
string arguments = anydeskId;
// Add --with-password flag if password is provided
bool hasPassword = !string.IsNullOrEmpty(_connectionInfo.Password);
@@ -242,27 +252,45 @@ namespace mRemoteNG.Connection.Protocol.AnyDesk
return false;
}
}
private bool IsValidAnydeskId(string anydeskId)
{
if (string.IsNullOrWhiteSpace(anydeskId))
return false;
// AnyDesk IDs can be:
// - Pure numeric (e.g., 123456789)
// - Alphanumeric with @ for aliases (e.g., alias@ad)
// - May contain hyphens and underscores in aliases
// Reject any characters that could be used for command injection
foreach (char c in anydeskId)
{
if (!char.IsLetterOrDigit(c) && c != '@' && c != '-' && c != '_' && c != '.')
{
return false;
}
}
return true;
}
private bool StartAnydeskWithPassword(string anydeskPath, string arguments)
{
try
{
// Use PowerShell to pipe the password to AnyDesk
// This is the recommended way according to AnyDesk documentation
string escapedPassword = _connectionInfo.Password.Replace("'", "''");
string powershellCommand = $"echo '{escapedPassword}' | & '{anydeskPath}' {arguments}";
// Start AnyDesk and pipe the password directly to stdin
// This avoids command injection vulnerabilities from using PowerShell
_process = new Process
{
StartInfo = new ProcessStartInfo
{
FileName = "powershell.exe",
Arguments = $"-WindowStyle Hidden -Command \"{powershellCommand}\"",
FileName = anydeskPath,
Arguments = arguments,
UseShellExecute = false,
CreateNoWindow = true,
CreateNoWindow = false,
RedirectStandardOutput = false,
RedirectStandardError = false,
RedirectStandardInput = false
RedirectStandardInput = true
},
EnableRaisingEvents = true
};
@@ -270,8 +298,23 @@ namespace mRemoteNG.Connection.Protocol.AnyDesk
_process.Exited += ProcessExited;
_process.Start();
// Write the password to stdin and close the stream
// AnyDesk expects the password on stdin when --with-password is used
try
{
if (_process.StandardInput != null)
{
_process.StandardInput.WriteLine(_connectionInfo.Password);
_process.StandardInput.Close();
}
}
catch (Exception ex)
{
Runtime.MessageCollector?.AddMessage(MessageClass.WarningMsg,
$"Failed to send password to AnyDesk: {ex.Message}", true);
}
// Wait for the AnyDesk window to appear
// Note: The window belongs to the AnyDesk process, not PowerShell
if (!WaitForAnydeskWindow())
{
Runtime.MessageCollector?.AddMessage(MessageClass.WarningMsg,

78
mRemoteNG/UI/Forms/FrmAbout.cs
View File

@@ -1,4 +1,5 @@
using System.Diagnostics;
using System;
using System.Diagnostics;
using System.Windows.Forms;
using mRemoteNG.App.Info;
using mRemoteNG.Themes;
@@ -77,29 +78,74 @@ namespace mRemoteNG.UI.Forms
private void OpenUrl(string url)
{
// Validate URL format to prevent injection
if (string.IsNullOrWhiteSpace(url))
return;
// Basic URL validation - ensure it starts with http:// or https://
if (!url.StartsWith("http://", StringComparison.OrdinalIgnoreCase) &&
!url.StartsWith("https://", StringComparison.OrdinalIgnoreCase))
{
// Invalid URL format - don't try to open it
return;
}
try
{
Process.Start(url);
// Use the standard .NET approach for opening URLs securely
// UseShellExecute=true delegates to the OS default handler
var startInfo = new ProcessStartInfo
{
FileName = url,
UseShellExecute = true
};
Process.Start(startInfo);
}
catch
{
// hack because of this: https://github.com/dotnet/corefx/issues/10361
if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
// Fallback for older .NET Core versions with bug: https://github.com/dotnet/corefx/issues/10361
// Use platform-specific URL launchers
try
{
url = url.Replace("&", "^&");
Process.Start(new ProcessStartInfo("cmd", $"/c start {url}") { CreateNoWindow = true });
if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
{
// Use rundll32 with url.dll as fallback
var startInfo = new ProcessStartInfo
{
FileName = "rundll32.exe",
UseShellExecute = false,
CreateNoWindow = true
};
startInfo.ArgumentList.Add("url.dll,FileProtocolHandler");
startInfo.ArgumentList.Add(url);
Process.Start(startInfo);
}
else if (RuntimeInformation.IsOSPlatform(OSPlatform.Linux))
{
var startInfo = new ProcessStartInfo
{
FileName = "xdg-open",
UseShellExecute = false
};
startInfo.ArgumentList.Add(url);
Process.Start(startInfo);
}
else if (RuntimeInformation.IsOSPlatform(OSPlatform.OSX))
{
var startInfo = new ProcessStartInfo
{
FileName = "open",
UseShellExecute = false
};
startInfo.ArgumentList.Add(url);
Process.Start(startInfo);
}
}
else if (RuntimeInformation.IsOSPlatform(OSPlatform.Linux))
catch
{
Process.Start("xdg-open", url);
}
else if (RuntimeInformation.IsOSPlatform(OSPlatform.OSX))
{
Process.Start("open", url);
}
else
{
throw;
// Unable to open URL - notify the user
Runtime.MessageCollector?.AddMessage(MessageClass.WarningMsg,
"Unable to open URL in browser. Please open manually: " + url, true);
}
}
}

7
mRemoteNG/UI/Forms/FrmUnhandledException.cs
View File

@@ -91,7 +91,12 @@ namespace mRemoteNG.UI.Forms
private void buttonCreateBug_Click(object sender, EventArgs e)
{
Process.Start(GeneralAppInfo.UrlBugs);
var startInfo = new ProcessStartInfo
{
FileName = GeneralAppInfo.UrlBugs,
UseShellExecute = true
};
Process.Start(startInfo);
}
}
}

41
mRemoteNG/UI/Forms/OptionsPages/NotificationsPage.cs
View File

@@ -7,6 +7,7 @@ using mRemoteNG.App;
using mRemoteNG.Config.Settings.Registry;
using mRemoteNG.Properties;
using mRemoteNG.Resources.Language;
using mRemoteNG.Tools;
namespace mRemoteNG.UI.Forms.OptionsPages
{
@@ -342,8 +343,17 @@ namespace mRemoteNG.UI.Forms.OptionsPages
{
try
{
// Validate path to prevent command injection
PathValidator.ValidatePathOrThrow(path, nameof(path));
// Open the file using the default application associated with its file type based on the user's preference
Process.Start(path);
// Use ProcessStartInfo with UseShellExecute for better control
var startInfo = new ProcessStartInfo
{
FileName = path,
UseShellExecute = true
};
Process.Start(startInfo);
return true;
}
catch
@@ -362,9 +372,19 @@ namespace mRemoteNG.UI.Forms.OptionsPages
{
try
{
// Validate path to prevent command injection
PathValidator.ValidatePathOrThrow(path, nameof(path));
// Open it in "Notepad" (Windows default editor).
// Usually available on all Windows systems
Process.Start("notepad.exe", path);
// Use ProcessStartInfo with ArgumentList for better security
var startInfo = new ProcessStartInfo
{
FileName = "notepad.exe",
UseShellExecute = false
};
startInfo.ArgumentList.Add(path);
Process.Start(startInfo);
return true;
}
catch
@@ -383,11 +403,22 @@ namespace mRemoteNG.UI.Forms.OptionsPages
{
try
{
// Validate path to prevent command injection
PathValidator.ValidatePathOrThrow(path, nameof(path));
// when all fails open filelocation to logfile...
// Open Windows Explorer to the directory containing the file
Process.Start("explorer.exe", $"/select,\"{path}\"");
return true;
}
// Explorer expects /select,"path" as a single argument
var startInfo = new ProcessStartInfo
{
FileName = "explorer.exe",
UseShellExecute = false
};
startInfo.ArgumentList.Add("/select,");
startInfo.ArgumentList.Add(path);
Process.Start(startInfo);
return true;
}
catch
{
// If necessary, the error can be logged here.

24
mRemoteNG/UI/Menu/msMain/HelpMenu.cs
View File

@@ -188,19 +188,29 @@ namespace mRemoteNG.UI.Menu
}
}
private void mMenInfoHelp_Click(object? sender, EventArgs e) => Process.Start("explorer.exe", GeneralAppInfo.UrlDocumentation);
private void mMenInfoHelp_Click(object? sender, EventArgs e) => OpenUrl(GeneralAppInfo.UrlDocumentation);
private void mMenInfoForum_Click(object? sender, EventArgs e) => Process.Start("explorer.exe", GeneralAppInfo.UrlForum);
private void mMenInfoForum_Click(object? sender, EventArgs e) => OpenUrl(GeneralAppInfo.UrlForum);
private void mMenInfoChat_Click(object? sender, EventArgs e) => Process.Start("explorer.exe", GeneralAppInfo.UrlChat);
private void mMenInfoChat_Click(object? sender, EventArgs e) => OpenUrl(GeneralAppInfo.UrlChat);
private void mMenInfoCommunity_Click(object? sender, EventArgs e) => Process.Start("explorer.exe", GeneralAppInfo.UrlCommunity);
private void mMenInfoCommunity_Click(object? sender, EventArgs e) => OpenUrl(GeneralAppInfo.UrlCommunity);
private void mMenInfoBug_Click(object? sender, EventArgs e) => Process.Start("explorer.exe", GeneralAppInfo.UrlBugs);
private void mMenInfoBug_Click(object? sender, EventArgs e) => OpenUrl(GeneralAppInfo.UrlBugs);
private void mMenInfoWebsite_Click(object? sender, EventArgs e) => Process.Start("explorer.exe", GeneralAppInfo.UrlHome);
private void mMenInfoWebsite_Click(object? sender, EventArgs e) => OpenUrl(GeneralAppInfo.UrlHome);
private void mMenInfoDonate_Click(object? sender, EventArgs e) => Process.Start("explorer.exe", GeneralAppInfo.UrlDonate);
private void mMenInfoDonate_Click(object? sender, EventArgs e) => OpenUrl(GeneralAppInfo.UrlDonate);
private static void OpenUrl(string url)
{
var startInfo = new ProcessStartInfo
{
FileName = url,
UseShellExecute = true
};
Process.Start(startInfo);
}
private void mMenInfoAbout_Click(object? sender, EventArgs e)
{

7
mRemoteNG/UI/Window/UpdateWindow.cs
View File

@@ -99,7 +99,12 @@ namespace mRemoteNG.UI.Window
return;
}
Process.Start(linkUri.ToString());
var startInfo = new ProcessStartInfo
{
FileName = linkUri.ToString(),
UseShellExecute = true
};
Process.Start(startInfo);
}
#endregion

204
mRemoteNGTests/Connection/Protocol/ProtocolAnydeskTests.cs Normal file
View File

@@ -0,0 +1,204 @@
using System;
using System.Reflection;
using mRemoteNG.Connection;
using mRemoteNG.Connection.Protocol.AnyDesk;
using NUnit.Framework;
namespace mRemoteNGTests.Connection.Protocol;
public class ProtocolAnyDeskTests
{
private ProtocolAnyDesk _protocolAnyDesk;
private ConnectionInfo _connectionInfo;
[SetUp]
public void Setup()
{
_connectionInfo = new ConnectionInfo();
_protocolAnyDesk = new ProtocolAnyDesk(_connectionInfo);
}
[TearDown]
public void Teardown()
{
_protocolAnyDesk?.Close();
_protocolAnyDesk = null;
_connectionInfo = null;
}
#region IsValidAnydeskId Tests
[Test]
public void IsValidAnydeskId_NumericId_ReturnsTrue()
{
// Valid numeric AnyDesk ID
bool result = InvokeIsValidAnydeskId("123456789");
Assert.That(result, Is.True);
}
[Test]
public void IsValidAnydeskId_AlphanumericWithAt_ReturnsTrue()
{
// Valid alias format: alias@ad
bool result = InvokeIsValidAnydeskId("myalias@ad");
Assert.That(result, Is.True);
}
[Test]
public void IsValidAnydeskId_WithHyphen_ReturnsTrue()
{
// Valid ID with hyphen
bool result = InvokeIsValidAnydeskId("my-alias@ad");
Assert.That(result, Is.True);
}
[Test]
public void IsValidAnydeskId_WithUnderscore_ReturnsTrue()
{
// Valid ID with underscore
bool result = InvokeIsValidAnydeskId("my_alias@ad");
Assert.That(result, Is.True);
}
[Test]
public void IsValidAnydeskId_WithDot_ReturnsTrue()
{
// Valid ID with dot
bool result = InvokeIsValidAnydeskId("alias.name@ad");
Assert.That(result, Is.True);
}
[Test]
public void IsValidAnydeskId_WithSemicolon_ReturnsFalse()
{
// Command injection attempt with semicolon
bool result = InvokeIsValidAnydeskId("123456789; calc.exe");
Assert.That(result, Is.False);
}
[Test]
public void IsValidAnydeskId_WithAmpersand_ReturnsFalse()
{
// Command injection attempt with ampersand
bool result = InvokeIsValidAnydeskId("123456789 & calc.exe");
Assert.That(result, Is.False);
}
[Test]
public void IsValidAnydeskId_WithPipe_ReturnsFalse()
{
// Command injection attempt with pipe
bool result = InvokeIsValidAnydeskId("123456789 | calc.exe");
Assert.That(result, Is.False);
}
[Test]
public void IsValidAnydeskId_WithRedirection_ReturnsFalse()
{
// Command injection attempt with redirection
bool result = InvokeIsValidAnydeskId("123456789 > output.txt");
Assert.That(result, Is.False);
}
[Test]
public void IsValidAnydeskId_WithBacktick_ReturnsFalse()
{
// PowerShell escape character
bool result = InvokeIsValidAnydeskId("123456789`calc");
Assert.That(result, Is.False);
}
[Test]
public void IsValidAnydeskId_WithDollarSign_ReturnsFalse()
{
// PowerShell variable indicator
bool result = InvokeIsValidAnydeskId("123456789$var");
Assert.That(result, Is.False);
}
[Test]
public void IsValidAnydeskId_WithParentheses_ReturnsFalse()
{
// Command substitution
bool result = InvokeIsValidAnydeskId("123456789(calc)");
Assert.That(result, Is.False);
}
[Test]
public void IsValidAnydeskId_WithNewline_ReturnsFalse()
{
// Newline injection
bool result = InvokeIsValidAnydeskId("123456789\ncalc");
Assert.That(result, Is.False);
}
[Test]
public void IsValidAnydeskId_WithCarriageReturn_ReturnsFalse()
{
// Carriage return injection
bool result = InvokeIsValidAnydeskId("123456789\rcalc");
Assert.That(result, Is.False);
}
[Test]
public void IsValidAnydeskId_WithQuotes_ReturnsFalse()
{
// Quote escape attempt
bool result = InvokeIsValidAnydeskId("123456789\"calc\"");
Assert.That(result, Is.False);
}
[Test]
public void IsValidAnydeskId_WithSingleQuotes_ReturnsFalse()
{
// Single quote escape attempt
bool result = InvokeIsValidAnydeskId("123456789'calc'");
Assert.That(result, Is.False);
}
[Test]
public void IsValidAnydeskId_EmptyString_ReturnsFalse()
{
// Empty string
bool result = InvokeIsValidAnydeskId("");
Assert.That(result, Is.False);
}
[Test]
public void IsValidAnydeskId_Whitespace_ReturnsFalse()
{
// Only whitespace
bool result = InvokeIsValidAnydeskId(" ");
Assert.That(result, Is.False);
}
[Test]
public void IsValidAnydeskId_Null_ReturnsFalse()
{
// Null string
bool result = InvokeIsValidAnydeskId(null);
Assert.That(result, Is.False);
}
#endregion
#region Helper Methods
/// <summary>
/// Uses reflection to invoke the private IsValidAnydeskId method
/// </summary>
private bool InvokeIsValidAnydeskId(string anydeskId)
{
var method = typeof(ProtocolAnyDesk).GetMethod("IsValidAnydeskId",
BindingFlags.NonPublic | BindingFlags.Instance);
if (method == null)
{
throw new Exception("IsValidAnydeskId method not found. The method may have been renamed or removed.");
}
return (bool)method.Invoke(_protocolAnydesk, new object[] { anydeskId });
}
#endregion
}