diff --git a/mRemoteNG/Config/Serializers/MiscSerializers/ActiveDirectoryDeserializer.cs b/mRemoteNG/Config/Serializers/MiscSerializers/ActiveDirectoryDeserializer.cs index f9ec3e7b8..c15805173 100644 --- a/mRemoteNG/Config/Serializers/MiscSerializers/ActiveDirectoryDeserializer.cs +++ b/mRemoteNG/Config/Serializers/MiscSerializers/ActiveDirectoryDeserializer.cs @@ -10,6 +10,7 @@ using mRemoteNG.Tools; using mRemoteNG.Tree; using mRemoteNG.Tree.Root; using mRemoteNG.Resources.Language; +using mRemoteNG.Security; using System.Runtime.Versioning; namespace mRemoteNG.Config.Serializers.MiscSerializers @@ -17,9 +18,48 @@ namespace mRemoteNG.Config.Serializers.MiscSerializers [SupportedOSPlatform("windows")] public class ActiveDirectoryDeserializer(string ldapPath, bool importSubOu) { - private readonly string _ldapPath = ldapPath.ThrowIfNullOrEmpty(nameof(ldapPath)); + private readonly string _ldapPath = SanitizeLdapPath(ldapPath.ThrowIfNullOrEmpty(nameof(ldapPath))); private readonly bool _importSubOu = importSubOu; + private static string SanitizeLdapPath(string ldapPath) + { + // Validate the LDAP path format + if (!LdapPathSanitizer.IsValidDistinguishedNameFormat(ldapPath)) + { + throw new ArgumentException("Invalid LDAP path format", nameof(ldapPath)); + } + + // For LDAP paths (URIs like LDAP://...), we need to sanitize the DN portion + // If it starts with LDAP:// or LDAPS://, extract and sanitize the DN part + if (ldapPath.StartsWith("LDAP://", StringComparison.OrdinalIgnoreCase) || + ldapPath.StartsWith("LDAPS://", StringComparison.OrdinalIgnoreCase)) + { + int schemeEndIndex = ldapPath.IndexOf("://", StringComparison.OrdinalIgnoreCase) + 3; + if (schemeEndIndex < ldapPath.Length) + { + // Find the server/domain part (before the first /) + int pathStartIndex = ldapPath.IndexOf('/', schemeEndIndex); + if (pathStartIndex > 0) + { + string scheme = ldapPath.Substring(0, schemeEndIndex); + string serverPart = ldapPath.Substring(schemeEndIndex, pathStartIndex - schemeEndIndex); + string dnPart = ldapPath.Substring(pathStartIndex + 1); + + // Sanitize the DN part + string sanitizedDn = LdapPathSanitizer.SanitizeDistinguishedName(dnPart); + return scheme + serverPart + "/" + sanitizedDn; + } + } + // If no DN part found, return the path as-is (just the server) + return ldapPath; + } + else + { + // For plain DN strings, sanitize directly + return LdapPathSanitizer.SanitizeDistinguishedName(ldapPath); + } + } + public ConnectionTreeModel Deserialize() { ConnectionTreeModel connectionTreeModel = new(); diff --git a/mRemoteNG/Security/LdapPathSanitizer.cs b/mRemoteNG/Security/LdapPathSanitizer.cs new file mode 100644 index 000000000..4e107d83e --- /dev/null +++ b/mRemoteNG/Security/LdapPathSanitizer.cs @@ -0,0 +1,166 @@ +using System; +using System.Text; + +namespace mRemoteNG.Security +{ + /// + /// Provides methods to sanitize LDAP distinguished names and filter values to prevent LDAP injection attacks. + /// Based on OWASP recommendations for LDAP injection prevention. + /// + public static class LdapPathSanitizer + { + /// + /// Sanitizes an LDAP distinguished name (DN) by escaping special characters. + /// This should be used for DN values like those passed to DirectoryEntry constructor. + /// + /// The DN to sanitize + /// A sanitized DN string safe from LDAP injection + public static string SanitizeDistinguishedName(string distinguishedName) + { + if (string.IsNullOrEmpty(distinguishedName)) + return distinguishedName; + + // Escape special characters in DN according to RFC 4514 + // Special characters that need escaping: , \ # + < > ; " = + // and leading/trailing spaces + StringBuilder result = new StringBuilder(); + + for (int i = 0; i < distinguishedName.Length; i++) + { + char c = distinguishedName[i]; + + // Escape special characters + switch (c) + { + case '\\': + result.Append("\\\\"); + break; + case ',': + result.Append("\\,"); + break; + case '+': + result.Append("\\+"); + break; + case '"': + result.Append("\\\""); + break; + case '<': + result.Append("\\<"); + break; + case '>': + result.Append("\\>"); + break; + case ';': + result.Append("\\;"); + break; + case '=': + result.Append("\\="); + break; + case '#': + // # needs to be escaped only if it's the first character + if (i == 0) + result.Append("\\#"); + else + result.Append(c); + break; + case ' ': + // Leading and trailing spaces need to be escaped + if (i == 0 || i == distinguishedName.Length - 1) + result.Append("\\ "); + else + result.Append(c); + break; + default: + // Check for control characters (0x00-0x1F and 0x7F) + if (c < 0x20 || c == 0x7F) + { + result.Append("\\"); + result.Append(((int)c).ToString("x2")); + } + else + { + result.Append(c); + } + break; + } + } + + return result.ToString(); + } + + /// + /// Sanitizes an LDAP filter value by escaping special characters. + /// This should be used for filter values in LDAP search filters. + /// + /// The filter value to sanitize + /// A sanitized filter value safe from LDAP injection + public static string SanitizeFilterValue(string filterValue) + { + if (string.IsNullOrEmpty(filterValue)) + return filterValue; + + // Escape special characters in filter according to RFC 4515 + // Special characters that need escaping: * ( ) \ NUL + StringBuilder result = new StringBuilder(); + + foreach (char c in filterValue) + { + switch (c) + { + case '\\': + result.Append("\\5c"); + break; + case '*': + result.Append("\\2a"); + break; + case '(': + result.Append("\\28"); + break; + case ')': + result.Append("\\29"); + break; + case '\0': + result.Append("\\00"); + break; + default: + // Check for other control characters + if (c < 0x20 || c == 0x7F) + { + result.Append("\\"); + result.Append(((int)c).ToString("x2")); + } + else + { + result.Append(c); + } + break; + } + } + + return result.ToString(); + } + + /// + /// Validates that a distinguished name appears to be in valid LDAP DN format. + /// This is a basic check to ensure the DN structure is reasonable. + /// + /// The DN to validate + /// True if the DN appears valid, false otherwise + public static bool IsValidDistinguishedNameFormat(string distinguishedName) + { + if (string.IsNullOrWhiteSpace(distinguishedName)) + return false; + + // Basic validation: should start with LDAP:// or contain = for attribute=value pairs + // This is a simple heuristic check, not a full RFC validation + if (distinguishedName.StartsWith("LDAP://", StringComparison.OrdinalIgnoreCase) || + distinguishedName.StartsWith("LDAPS://", StringComparison.OrdinalIgnoreCase)) + { + return true; + } + + // Check if it looks like a DN (contains attribute=value pairs) + return distinguishedName.Contains("="); + } + } +} diff --git a/mRemoteNG/Tools/ADhelper.cs b/mRemoteNG/Tools/ADhelper.cs index 6e4d6a3ee..477662909 100644 --- a/mRemoteNG/Tools/ADhelper.cs +++ b/mRemoteNG/Tools/ADhelper.cs @@ -3,6 +3,7 @@ using System.Collections; using System.DirectoryServices; using System.Runtime.InteropServices; using System.Runtime.Versioning; +using mRemoteNG.Security; namespace mRemoteNG.Tools { @@ -17,9 +18,13 @@ namespace mRemoteNG.Tools public void GetChildEntries(string adPath = "") { - _dEntry = adPath.Length <= 0 - ? Domain.Length <= 0 ? new DirectoryEntry() : new DirectoryEntry("LDAP://" + Domain) - : new DirectoryEntry(adPath); + // Sanitize inputs to prevent LDAP injection + string sanitizedDomain = string.IsNullOrEmpty(Domain) ? string.Empty : LdapPathSanitizer.SanitizeDistinguishedName(Domain); + string sanitizedAdPath = string.IsNullOrEmpty(adPath) ? string.Empty : SanitizeLdapPath(adPath); + + _dEntry = sanitizedAdPath.Length <= 0 + ? sanitizedDomain.Length <= 0 ? new DirectoryEntry() : new DirectoryEntry("LDAP://" + sanitizedDomain) + : new DirectoryEntry(sanitizedAdPath); try { foreach (DirectoryEntry child in _dEntry.Children) @@ -31,5 +36,43 @@ namespace mRemoteNG.Tools throw new Exception("Could not find AD Server", ex); } } + + private static string SanitizeLdapPath(string ldapPath) + { + // Validate the LDAP path format + if (!LdapPathSanitizer.IsValidDistinguishedNameFormat(ldapPath)) + { + throw new ArgumentException("Invalid LDAP path format", nameof(ldapPath)); + } + + // For LDAP paths (URIs like LDAP://...), we need to sanitize the DN portion + if (ldapPath.StartsWith("LDAP://", StringComparison.OrdinalIgnoreCase) || + ldapPath.StartsWith("LDAPS://", StringComparison.OrdinalIgnoreCase)) + { + int schemeEndIndex = ldapPath.IndexOf("://", StringComparison.OrdinalIgnoreCase) + 3; + if (schemeEndIndex < ldapPath.Length) + { + // Find the server/domain part (before the first /) + int pathStartIndex = ldapPath.IndexOf('/', schemeEndIndex); + if (pathStartIndex > 0) + { + string scheme = ldapPath.Substring(0, schemeEndIndex); + string serverPart = ldapPath.Substring(schemeEndIndex, pathStartIndex - schemeEndIndex); + string dnPart = ldapPath.Substring(pathStartIndex + 1); + + // Sanitize the DN part + string sanitizedDn = LdapPathSanitizer.SanitizeDistinguishedName(dnPart); + return scheme + serverPart + "/" + sanitizedDn; + } + } + // If no DN part found, return the path as-is (just the server) + return ldapPath; + } + else + { + // For plain DN strings, sanitize directly + return LdapPathSanitizer.SanitizeDistinguishedName(ldapPath); + } + } } } \ No newline at end of file diff --git a/mRemoteNGTests/Security/LdapPathSanitizerTests.cs b/mRemoteNGTests/Security/LdapPathSanitizerTests.cs new file mode 100644 index 000000000..cce31cc2f --- /dev/null +++ b/mRemoteNGTests/Security/LdapPathSanitizerTests.cs @@ -0,0 +1,231 @@ +using mRemoteNG.Security; +using NUnit.Framework; + +namespace mRemoteNGTests.Security +{ + [TestFixture] + public class LdapPathSanitizerTests + { + [Test] + public void SanitizeDistinguishedName_EscapesBackslash() + { + string input = "CN=Test\\User"; + string result = LdapPathSanitizer.SanitizeDistinguishedName(input); + Assert.That(result, Is.EqualTo("CN\\=Test\\\\User")); + } + + [Test] + public void SanitizeDistinguishedName_EscapesComma() + { + string input = "CN=User,Test"; + string result = LdapPathSanitizer.SanitizeDistinguishedName(input); + Assert.That(result, Is.EqualTo("CN\\=User\\,Test")); + } + + [Test] + public void SanitizeDistinguishedName_EscapesPlus() + { + string input = "CN=User+Admin"; + string result = LdapPathSanitizer.SanitizeDistinguishedName(input); + Assert.That(result, Is.EqualTo("CN\\=User\\+Admin")); + } + + [Test] + public void SanitizeDistinguishedName_EscapesQuotes() + { + string input = "CN=\"Test User\""; + string result = LdapPathSanitizer.SanitizeDistinguishedName(input); + Assert.That(result, Is.EqualTo("CN\\=\\\"Test User\\\"")); + } + + [Test] + public void SanitizeDistinguishedName_EscapesLessThan() + { + string input = "CN="; + string result = LdapPathSanitizer.SanitizeDistinguishedName(input); + Assert.That(result, Is.EqualTo("CN\\=\\")); + } + + [Test] + public void SanitizeDistinguishedName_EscapesSemicolon() + { + string input = "CN=User;Admin"; + string result = LdapPathSanitizer.SanitizeDistinguishedName(input); + Assert.That(result, Is.EqualTo("CN\\=User\\;Admin")); + } + + [Test] + public void SanitizeDistinguishedName_EscapesLeadingHash() + { + string input = "#Test"; + string result = LdapPathSanitizer.SanitizeDistinguishedName(input); + Assert.That(result, Is.EqualTo("\\#Test")); + } + + [Test] + public void SanitizeDistinguishedName_DoesNotEscapeMiddleHash() + { + string input = "Test#User"; + string result = LdapPathSanitizer.SanitizeDistinguishedName(input); + Assert.That(result, Is.EqualTo("Test#User")); + } + + [Test] + public void SanitizeDistinguishedName_EscapesLeadingSpace() + { + string input = " Test"; + string result = LdapPathSanitizer.SanitizeDistinguishedName(input); + Assert.That(result, Is.EqualTo("\\ Test")); + } + + [Test] + public void SanitizeDistinguishedName_EscapesTrailingSpace() + { + string input = "Test "; + string result = LdapPathSanitizer.SanitizeDistinguishedName(input); + Assert.That(result, Is.EqualTo("Test\\ ")); + } + + [Test] + public void SanitizeDistinguishedName_DoesNotEscapeMiddleSpace() + { + string input = "Test User"; + string result = LdapPathSanitizer.SanitizeDistinguishedName(input); + Assert.That(result, Is.EqualTo("Test User")); + } + + [Test] + public void SanitizeDistinguishedName_ReturnsEmptyForNull() + { + string result = LdapPathSanitizer.SanitizeDistinguishedName(null); + Assert.That(result, Is.Null); + } + + [Test] + public void SanitizeDistinguishedName_ReturnsEmptyForEmpty() + { + string result = LdapPathSanitizer.SanitizeDistinguishedName(string.Empty); + Assert.That(result, Is.EqualTo(string.Empty)); + } + + [Test] + public void SanitizeFilterValue_EscapesAsterisk() + { + string input = "user*"; + string result = LdapPathSanitizer.SanitizeFilterValue(input); + Assert.That(result, Is.EqualTo("user\\2a")); + } + + [Test] + public void SanitizeFilterValue_EscapesParentheses() + { + string input = "(admin)"; + string result = LdapPathSanitizer.SanitizeFilterValue(input); + Assert.That(result, Is.EqualTo("\\28admin\\29")); + } + + [Test] + public void SanitizeFilterValue_EscapesBackslash() + { + string input = "user\\admin"; + string result = LdapPathSanitizer.SanitizeFilterValue(input); + Assert.That(result, Is.EqualTo("user\\5cadmin")); + } + + [Test] + public void SanitizeFilterValue_EscapesNullCharacter() + { + string input = "user\0admin"; + string result = LdapPathSanitizer.SanitizeFilterValue(input); + Assert.That(result, Is.EqualTo("user\\00admin")); + } + + [Test] + public void SanitizeFilterValue_ReturnsNullForNull() + { + string result = LdapPathSanitizer.SanitizeFilterValue(null); + Assert.That(result, Is.Null); + } + + [Test] + public void SanitizeFilterValue_ReturnsEmptyForEmpty() + { + string result = LdapPathSanitizer.SanitizeFilterValue(string.Empty); + Assert.That(result, Is.EqualTo(string.Empty)); + } + + [Test] + public void IsValidDistinguishedNameFormat_ReturnsTrueForLdapUri() + { + string input = "LDAP://dc=example,dc=com"; + bool result = LdapPathSanitizer.IsValidDistinguishedNameFormat(input); + Assert.That(result, Is.True); + } + + [Test] + public void IsValidDistinguishedNameFormat_ReturnsTrueForLdapsUri() + { + string input = "LDAPS://dc=example,dc=com"; + bool result = LdapPathSanitizer.IsValidDistinguishedNameFormat(input); + Assert.That(result, Is.True); + } + + [Test] + public void IsValidDistinguishedNameFormat_ReturnsTrueForDnWithEquals() + { + string input = "CN=User,OU=Users,DC=example,DC=com"; + bool result = LdapPathSanitizer.IsValidDistinguishedNameFormat(input); + Assert.That(result, Is.True); + } + + [Test] + public void IsValidDistinguishedNameFormat_ReturnsFalseForNull() + { + bool result = LdapPathSanitizer.IsValidDistinguishedNameFormat(null); + Assert.That(result, Is.False); + } + + [Test] + public void IsValidDistinguishedNameFormat_ReturnsFalseForEmpty() + { + bool result = LdapPathSanitizer.IsValidDistinguishedNameFormat(string.Empty); + Assert.That(result, Is.False); + } + + [Test] + public void IsValidDistinguishedNameFormat_ReturnsFalseForWhitespace() + { + bool result = LdapPathSanitizer.IsValidDistinguishedNameFormat(" "); + Assert.That(result, Is.False); + } + + [Test] + public void IsValidDistinguishedNameFormat_ReturnsFalseForPlainString() + { + bool result = LdapPathSanitizer.IsValidDistinguishedNameFormat("plainstring"); + Assert.That(result, Is.False); + } + + [Test] + public void SanitizeDistinguishedName_PreventsLdapInjectionAttempt() + { + // Test common LDAP injection patterns + string input = "*)(objectClass=*))(|(cn=*"; + string result = LdapPathSanitizer.SanitizeDistinguishedName(input); + // Verify special characters are escaped + Assert.That(result, Does.Contain("\\=")); + } + + [Test] + public void SanitizeFilterValue_PreventsLdapInjectionAttempt() + { + // Test common LDAP injection patterns + string input = "*)(uid=*))(|(uid=*"; + string result = LdapPathSanitizer.SanitizeFilterValue(input); + // Verify special characters are escaped + Assert.That(result, Does.Contain("\\2a")); + Assert.That(result, Does.Contain("\\28")); + Assert.That(result, Does.Contain("\\29")); + } + } +}