Add SecureXmlHelper and update all XML deserialization to prevent XXE attacks

Co-authored-by: Kvarkas <3611964+Kvarkas@users.noreply.github.com>
This commit is contained in:
copilot-swe-agent[bot]
2025-10-07 15:20:14 +00:00
parent 4ae1281a3a
commit 26bc38cf8c
8 changed files with 86 additions and 16 deletions

View File

@@ -94,8 +94,7 @@ namespace mRemoteNG.Config.Serializers.ConnectionSerializers.Xml
connections = _decryptor.LegacyFullFileDecrypt(connections);
if (connections != "")
{
_xmlDocument = new XmlDocument();
_xmlDocument.LoadXml(connections);
_xmlDocument = SecureXmlHelper.LoadXmlFromString(connections);
}
}

View File

@@ -20,8 +20,7 @@ namespace mRemoteNG.Config.Serializers.MiscSerializers
RootNodeInfo root = new(RootNodeType.Connection);
connectionTreeModel.AddRootNode(root);
XmlDocument xmlDocument = new();
xmlDocument.LoadXml(puttycmConnectionsXml);
XmlDocument xmlDocument = SecureXmlHelper.LoadXmlFromString(puttycmConnectionsXml);
XmlNode configurationNode = xmlDocument.SelectSingleNode("/configuration");

View File

@@ -26,9 +26,7 @@ namespace mRemoteNG.Config.Serializers.MiscSerializers
ConnectionTreeModel connectionTreeModel = new();
RootNodeInfo root = new(RootNodeType.Connection);
XmlDocument xmlDocument = new();
xmlDocument.LoadXml(rdcmConnectionsXml);
XmlDocument xmlDocument = SecureXmlHelper.LoadXmlFromString(rdcmConnectionsXml);
XmlNode rdcManNode = xmlDocument.SelectSingleNode("/RDCMan");
VerifySchemaVersion(rdcManNode);

View File

@@ -2,6 +2,7 @@
using mRemoteNG.Connection;
using mRemoteNG.Connection.Protocol;
using mRemoteNG.Container;
using mRemoteNG.Security;
using mRemoteNG.Tree;
using mRemoteNG.Tree.Root;
using System;
@@ -22,8 +23,7 @@ namespace mRemoteNG.Config.Serializers.MiscSerializers
RootNodeInfo root = new(RootNodeType.Connection);
connectionTreeModel.AddRootNode(root);
XmlDocument xmlDocument = new();
xmlDocument.LoadXml(content);
XmlDocument xmlDocument = SecureXmlHelper.LoadXmlFromString(content);
XmlNode sessionsNode = xmlDocument.SelectSingleNode("/VanDyke/key[@name=\"Sessions\"]");

View File

@@ -5,6 +5,7 @@ using mRemoteNG.UI.Forms;
using System.IO;
using System.Xml;
using mRemoteNG.Messages;
using mRemoteNG.Security;
using mRemoteNG.Tools;
using mRemoteNG.UI.Controls;
using System.Runtime.Versioning;
@@ -40,18 +41,18 @@ namespace mRemoteNG.Config.Settings
Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.LocalApplicationData), GeneralAppInfo.ProductName, SettingsFileInfo.ExtAppsFilesName);
#endif
string newPath = Path.Combine(SettingsFileInfo.SettingsPath, SettingsFileInfo.ExtAppsFilesName);
XmlDocument xDom = new();
XmlDocument xDom;
if (File.Exists(newPath))
{
_messageCollector.AddMessage(MessageClass.InformationMsg, $"Loading External Apps from: {newPath}",
true);
xDom.Load(newPath);
xDom = SecureXmlHelper.LoadXmlFromFile(newPath);
}
#if !PORTABLE
else if (File.Exists(oldPath))
{
_messageCollector.AddMessage(MessageClass.InformationMsg, $"Loading External Apps from: {oldPath}", true);
xDom.Load(oldPath);
xDom = SecureXmlHelper.LoadXmlFromFile(oldPath);
}
#endif

View File

@@ -31,6 +31,7 @@ using System.Windows.Forms;
using System.Collections.Specialized;
using System.Xml;
using System.IO;
using mRemoteNG.Security;
//using mRemoteNG.App;
@@ -61,8 +62,7 @@ namespace mRemoteNG.Config.Settings.Providers
if (_xmlDocument != null) return _xmlDocument;
try
{
_xmlDocument = new XmlDocument();
_xmlDocument.Load(_filePath);
_xmlDocument = SecureXmlHelper.LoadXmlFromFile(_filePath);
}
catch (Exception /*ex*/)
{

View File

@@ -0,0 +1,73 @@
using System.IO;
using System.Xml;
namespace mRemoteNG.Security
{
/// <summary>
/// Helper class for securely loading XML documents to prevent XXE (XML External Entity) attacks
/// </summary>
public static class SecureXmlHelper
{
/// <summary>
/// Creates an XmlDocument with secure settings that prevent XXE attacks
/// </summary>
/// <returns>A new XmlDocument with secure settings</returns>
public static XmlDocument CreateSecureXmlDocument()
{
XmlDocument xmlDocument = new XmlDocument
{
XmlResolver = null // Disable external entity resolution
};
return xmlDocument;
}
/// <summary>
/// Safely loads XML content from a string into an XmlDocument with XXE protection
/// </summary>
/// <param name="xmlContent">The XML content to load</param>
/// <returns>An XmlDocument with the loaded content</returns>
public static XmlDocument LoadXmlFromString(string xmlContent)
{
XmlReaderSettings settings = CreateSecureReaderSettings();
using (StringReader stringReader = new StringReader(xmlContent))
using (XmlReader reader = XmlReader.Create(stringReader, settings))
{
XmlDocument xmlDocument = CreateSecureXmlDocument();
xmlDocument.Load(reader);
return xmlDocument;
}
}
/// <summary>
/// Safely loads XML content from a file into an XmlDocument with XXE protection
/// </summary>
/// <param name="filePath">The path to the XML file</param>
/// <returns>An XmlDocument with the loaded content</returns>
public static XmlDocument LoadXmlFromFile(string filePath)
{
XmlReaderSettings settings = CreateSecureReaderSettings();
using (XmlReader reader = XmlReader.Create(filePath, settings))
{
XmlDocument xmlDocument = CreateSecureXmlDocument();
xmlDocument.Load(reader);
return xmlDocument;
}
}
/// <summary>
/// Creates XmlReaderSettings with secure defaults to prevent XXE attacks
/// </summary>
/// <returns>XmlReaderSettings configured for security</returns>
private static XmlReaderSettings CreateSecureReaderSettings()
{
return new XmlReaderSettings
{
DtdProcessing = DtdProcessing.Prohibit, // Prohibit DTD processing
XmlResolver = null, // Disable external entity resolution
MaxCharactersFromEntities = 0, // Don't allow entity expansion
IgnoreProcessingInstructions = true, // Ignore processing instructions
IgnoreComments = true // Ignore comments
};
}
}
}

View File

@@ -3,6 +3,7 @@ using System.Drawing;
using System.Globalization;
using System.IO;
using System.Xml;
using mRemoteNG.Security;
using mRemoteNG.Themes;
namespace mRemoteNG.Themes
@@ -17,8 +18,7 @@ namespace mRemoteNG.Themes
//warning, defaultpalette should always contain all the values, because when is loaded there is no default palette (parameter is null
public MremoteNGPaletteManipulator(byte[] file, ExtendedColorPalette defaultPalette = null)
{
_xml = new XmlDocument();
_xml.LoadXml(new StreamReader(new MemoryStream(file)).ReadToEnd());
_xml = SecureXmlHelper.LoadXmlFromString(new StreamReader(new MemoryStream(file)).ReadToEnd());
_defaultPalette = defaultPalette ?? new ExtendedColorPalette();
}