diff --git a/mRemoteNG/App/Shutdown.cs b/mRemoteNG/App/Shutdown.cs index f577c8f4..c27dd238 100644 --- a/mRemoteNG/App/Shutdown.cs +++ b/mRemoteNG/App/Shutdown.cs @@ -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 }); + } } } } \ No newline at end of file diff --git a/mRemoteNG/Connection/Protocol/AnyDesk/ProtocolAnyDesk.cs b/mRemoteNG/Connection/Protocol/AnyDesk/ProtocolAnyDesk.cs index e6df6a9f..3206d5b8 100644 --- a/mRemoteNG/Connection/Protocol/AnyDesk/ProtocolAnyDesk.cs +++ b/mRemoteNG/Connection/Protocol/AnyDesk/ProtocolAnyDesk.cs @@ -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, diff --git a/mRemoteNG/UI/Forms/FrmAbout.cs b/mRemoteNG/UI/Forms/FrmAbout.cs index 06e8be28..969c1879 100644 --- a/mRemoteNG/UI/Forms/FrmAbout.cs +++ b/mRemoteNG/UI/Forms/FrmAbout.cs @@ -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 { diff --git a/mRemoteNG/UI/Forms/FrmUnhandledException.cs b/mRemoteNG/UI/Forms/FrmUnhandledException.cs index c42e27cd..8f431060 100644 --- a/mRemoteNG/UI/Forms/FrmUnhandledException.cs +++ b/mRemoteNG/UI/Forms/FrmUnhandledException.cs @@ -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); } } } \ No newline at end of file diff --git a/mRemoteNG/UI/Forms/OptionsPages/NotificationsPage.cs b/mRemoteNG/UI/Forms/OptionsPages/NotificationsPage.cs index a2701cf7..5bda1518 100644 --- a/mRemoteNG/UI/Forms/OptionsPages/NotificationsPage.cs +++ b/mRemoteNG/UI/Forms/OptionsPages/NotificationsPage.cs @@ -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 diff --git a/mRemoteNG/UI/Menu/msMain/HelpMenu.cs b/mRemoteNG/UI/Menu/msMain/HelpMenu.cs index e948b53e..89adeb9f 100644 --- a/mRemoteNG/UI/Menu/msMain/HelpMenu.cs +++ b/mRemoteNG/UI/Menu/msMain/HelpMenu.cs @@ -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) { diff --git a/mRemoteNG/UI/Window/UpdateWindow.cs b/mRemoteNG/UI/Window/UpdateWindow.cs index d590152d..93594020 100644 --- a/mRemoteNG/UI/Window/UpdateWindow.cs +++ b/mRemoteNG/UI/Window/UpdateWindow.cs @@ -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