mirror of
https://github.com/mRemoteNG/mRemoteNG.git
synced 2026-02-17 22:11:48 +08:00
Merge pull request #3064 from mRemoteNG/copilot/fix-ldap-query-injection
Fix LDAP injection vulnerability in Active Directory integration
This commit is contained in:
@@ -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();
|
||||
|
||||
166
mRemoteNG/Security/LdapPathSanitizer.cs
Normal file
166
mRemoteNG/Security/LdapPathSanitizer.cs
Normal file
@@ -0,0 +1,166 @@
|
||||
using System;
|
||||
using System.Text;
|
||||
|
||||
namespace mRemoteNG.Security
|
||||
{
|
||||
/// <summary>
|
||||
/// Provides methods to sanitize LDAP distinguished names and filter values to prevent LDAP injection attacks.
|
||||
/// Based on OWASP recommendations for LDAP injection prevention.
|
||||
/// </summary>
|
||||
public static class LdapPathSanitizer
|
||||
{
|
||||
/// <summary>
|
||||
/// Sanitizes an LDAP distinguished name (DN) by escaping special characters.
|
||||
/// This should be used for DN values like those passed to DirectoryEntry constructor.
|
||||
/// </summary>
|
||||
/// <param name="distinguishedName">The DN to sanitize</param>
|
||||
/// <returns>A sanitized DN string safe from LDAP injection</returns>
|
||||
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();
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Sanitizes an LDAP filter value by escaping special characters.
|
||||
/// This should be used for filter values in LDAP search filters.
|
||||
/// </summary>
|
||||
/// <param name="filterValue">The filter value to sanitize</param>
|
||||
/// <returns>A sanitized filter value safe from LDAP injection</returns>
|
||||
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();
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// 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.
|
||||
/// </summary>
|
||||
/// <param name="distinguishedName">The DN to validate</param>
|
||||
/// <returns>True if the DN appears valid, false otherwise</returns>
|
||||
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("=");
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -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);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
231
mRemoteNGTests/Security/LdapPathSanitizerTests.cs
Normal file
231
mRemoteNGTests/Security/LdapPathSanitizerTests.cs
Normal file
@@ -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=<User>";
|
||||
string result = LdapPathSanitizer.SanitizeDistinguishedName(input);
|
||||
Assert.That(result, Is.EqualTo("CN\\=\\<User\\>"));
|
||||
}
|
||||
|
||||
[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"));
|
||||
}
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user