mirror of
https://github.com/mRemoteNG/mRemoteNG.git
synced 2026-02-17 22:11:48 +08:00
Fix command injection vulnerabilities in Process.Start calls
Co-authored-by: Kvarkas <3611964+Kvarkas@users.noreply.github.com>
This commit is contained in:
@@ -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 });
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -211,6 +211,16 @@ 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();
|
||||
|
||||
// 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
|
||||
@@ -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,
|
||||
|
||||
@@ -79,23 +79,50 @@ namespace mRemoteNG.UI.Forms
|
||||
{
|
||||
try
|
||||
{
|
||||
Process.Start(url);
|
||||
// Try to open URL with UseShellExecute
|
||||
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))
|
||||
{
|
||||
url = url.Replace("&", "^&");
|
||||
Process.Start(new ProcessStartInfo("cmd", $"/c start {url}") { CreateNoWindow = true });
|
||||
// Use ArgumentList for better security instead of string concatenation
|
||||
var startInfo = new ProcessStartInfo
|
||||
{
|
||||
FileName = "cmd",
|
||||
UseShellExecute = false,
|
||||
CreateNoWindow = true
|
||||
};
|
||||
startInfo.ArgumentList.Add("/c");
|
||||
startInfo.ArgumentList.Add("start");
|
||||
startInfo.ArgumentList.Add(url);
|
||||
Process.Start(startInfo);
|
||||
}
|
||||
else if (RuntimeInformation.IsOSPlatform(OSPlatform.Linux))
|
||||
{
|
||||
Process.Start("xdg-open", url);
|
||||
var startInfo = new ProcessStartInfo
|
||||
{
|
||||
FileName = "xdg-open",
|
||||
UseShellExecute = false
|
||||
};
|
||||
startInfo.ArgumentList.Add(url);
|
||||
Process.Start(startInfo);
|
||||
}
|
||||
else if (RuntimeInformation.IsOSPlatform(OSPlatform.OSX))
|
||||
{
|
||||
Process.Start("open", url);
|
||||
var startInfo = new ProcessStartInfo
|
||||
{
|
||||
FileName = "open",
|
||||
UseShellExecute = false
|
||||
};
|
||||
startInfo.ArgumentList.Add(url);
|
||||
Process.Start(startInfo);
|
||||
}
|
||||
else
|
||||
{
|
||||
|
||||
@@ -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);
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -342,8 +342,17 @@ namespace mRemoteNG.UI.Forms.OptionsPages
|
||||
{
|
||||
try
|
||||
{
|
||||
// Validate path to prevent command injection
|
||||
Tools.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 +371,19 @@ namespace mRemoteNG.UI.Forms.OptionsPages
|
||||
{
|
||||
try
|
||||
{
|
||||
// Validate path to prevent command injection
|
||||
Tools.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,9 +402,19 @@ namespace mRemoteNG.UI.Forms.OptionsPages
|
||||
{
|
||||
try
|
||||
{
|
||||
// Validate path to prevent command injection
|
||||
Tools.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}\"");
|
||||
// Use ArgumentList for better security
|
||||
var startInfo = new ProcessStartInfo
|
||||
{
|
||||
FileName = "explorer.exe",
|
||||
UseShellExecute = false
|
||||
};
|
||||
startInfo.ArgumentList.Add("/select," + path);
|
||||
Process.Start(startInfo);
|
||||
return true;
|
||||
}
|
||||
catch
|
||||
|
||||
@@ -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)
|
||||
{
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user