Skip to content

Commit

Permalink
Bed-4487: fix handling for unicode control characters (#141)
Browse files Browse the repository at this point in the history 
* fix: handling for control characters

* chore: update tests

* fix: it works!

* chore: cleanup

* fix: feedback + swap to HashSet

* chore: use static initializer

* chore: reformat

* fix: use unionwith instead of intersectwith

* chore: fix tests

* chore: use sidbytes

---------

Co-authored-by: rvazarkar <rvazarkar@specterops.io>
  • Loading branch information
@mistahj67 @rvazarkar
mistahj67 and rvazarkar authored Jul 26, 2024
1 parent 8102ec1 commit 15553c2
Show file tree
Hide file tree
Showing 4 changed files with 142 additions and 28 deletions.
1 change: 1 addition & 0 deletions src/CommonLib/Enums/LDAPProperties.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,5 +80,6 @@ public static class LDAPProperties
public const string ServerName = "servername";
public const string OU = "ou";
public const string ProfilePath = "profilepath";
public const string DSASignature = "dsasignature";
}
}
90 changes: 67 additions & 23 deletions src/CommonLib/Processors/LdapPropertyProcessor.cs
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
using System;
using System;
using System.Collections.Generic;
using System.Diagnostics.CodeAnalysis;
using System.Linq;
Expand All @@ -11,16 +11,27 @@
using SharpHoundCommonLib.Enums;
using SharpHoundCommonLib.LDAPQueries;
using SharpHoundCommonLib.OutputTypes;

// ReSharper disable StringLiteralTypo

namespace SharpHoundCommonLib.Processors {
public class LdapPropertyProcessor {
private static readonly string[] ReservedAttributes = CommonProperties.TypeResolutionProps
.Concat(CommonProperties.BaseQueryProps).Concat(CommonProperties.GroupResolutionProps)
.Concat(CommonProperties.ComputerMethodProps).Concat(CommonProperties.ACLProps)
.Concat(CommonProperties.ObjectPropsProps).Concat(CommonProperties.ContainerProps)
.Concat(CommonProperties.SPNTargetProps).Concat(CommonProperties.DomainTrustProps)
.Concat(CommonProperties.GPOLocalGroupProps).Concat(CommonProperties.CertAbuseProps).ToArray();
private static readonly HashSet<string> ReservedAttributes = new();

static LdapPropertyProcessor() {
ReservedAttributes.UnionWith(CommonProperties.TypeResolutionProps);
ReservedAttributes.UnionWith(CommonProperties.BaseQueryProps);
ReservedAttributes.UnionWith(CommonProperties.GroupResolutionProps);
ReservedAttributes.UnionWith(CommonProperties.ComputerMethodProps);
ReservedAttributes.UnionWith(CommonProperties.ACLProps);
ReservedAttributes.UnionWith(CommonProperties.ObjectPropsProps);
ReservedAttributes.UnionWith(CommonProperties.ContainerProps);
ReservedAttributes.UnionWith(CommonProperties.SPNTargetProps);
ReservedAttributes.UnionWith(CommonProperties.DomainTrustProps);
ReservedAttributes.UnionWith(CommonProperties.GPOLocalGroupProps);
ReservedAttributes.UnionWith(CommonProperties.CertAbuseProps);
ReservedAttributes.Add(LDAPProperties.DSASignature);
}

private readonly ILdapUtils _utils;

Expand Down Expand Up @@ -176,7 +187,7 @@ public async Task<UserProperties> ReadUserProperties(IDirectoryObject entry, str
}

props.Add("lastlogon", Helpers.ConvertFileTimeToUnixEpoch(lastLogon));

if (!entry.TryGetProperty(LDAPProperties.LastLogonTimestamp, out var lastLogonTimeStamp)) {
lastLogonTimeStamp = null;
}
Expand All @@ -186,6 +197,7 @@ public async Task<UserProperties> ReadUserProperties(IDirectoryObject entry, str
if (!entry.TryGetProperty(LDAPProperties.PasswordLastSet, out var passwordLastSet)) {
passwordLastSet = null;
}

props.Add("pwdlastset",
Helpers.ConvertFileTimeToUnixEpoch(passwordLastSet));
entry.TryGetArrayProperty(LDAPProperties.ServicePrincipalNames, out var spn);
Expand Down Expand Up @@ -254,7 +266,7 @@ public async Task<ComputerProperties> ReadComputerProperties(IDirectoryObject en
props.Add("unconstraineddelegation", flags.HasFlag(UacFlags.TrustedForDelegation));
props.Add("trustedtoauth", flags.HasFlag(UacFlags.TrustedToAuthForDelegation));
props.Add("isdc", flags.HasFlag(UacFlags.ServerTrustAccount));

var comps = new List<TypedPrincipal>();
if (flags.HasFlag(UacFlags.TrustedToAuthForDelegation) &&
entry.TryGetArrayProperty(LDAPProperties.AllowedToDelegateTo, out var delegates)) {
Expand Down Expand Up @@ -355,7 +367,7 @@ public static Dictionary<string, object> ReadRootCAProperties(IDirectoryObject e
props.Add("hasbasicconstraints", cert.HasBasicConstraints);
props.Add("basicconstraintpathlength", cert.BasicConstraintPathLength);
}

return props;
}

Expand All @@ -366,7 +378,7 @@ public static Dictionary<string, object> ReadRootCAProperties(IDirectoryObject e
/// <returns>Returns a dictionary with the common properties and the crosscertificatepair property of the AICA</returns>
public static Dictionary<string, object> ReadAIACAProperties(IDirectoryObject entry) {
var props = GetCommonProps(entry);
entry.TryGetByteArrayProperty(LDAPProperties.CrossCertificatePair, out var crossCertificatePair);
entry.TryGetByteArrayProperty(LDAPProperties.CrossCertificatePair, out var crossCertificatePair);
var hasCrossCertificatePair = crossCertificatePair.Length > 0;

props.Add("crosscertificatepair", crossCertificatePair);
Expand All @@ -387,7 +399,8 @@ public static Dictionary<string, object> ReadAIACAProperties(IDirectoryObject en

public static Dictionary<string, object> ReadEnterpriseCAProperties(IDirectoryObject entry) {
var props = GetCommonProps(entry);
if (entry.TryGetIntProperty("flags", out var flags)) props.Add("flags", (PKICertificateAuthorityFlags)flags);
if (entry.TryGetIntProperty("flags", out var flags))
props.Add("flags", (PKICertificateAuthorityFlags)flags);
props.Add("caname", entry.GetProperty(LDAPProperties.Name));
props.Add("dnshostname", entry.GetProperty(LDAPProperties.DNSHostName));

Expand Down Expand Up @@ -461,7 +474,8 @@ public static Dictionary<string, object> ReadCertTemplateProperties(IDirectoryOb

entry.TryGetArrayProperty(LDAPProperties.ExtendedKeyUsage, out var ekus);
props.Add("ekus", ekus);
entry.TryGetArrayProperty(LDAPProperties.CertificateApplicationPolicy, out var certificateApplicationPolicy);
entry.TryGetArrayProperty(LDAPProperties.CertificateApplicationPolicy,
out var certificateApplicationPolicy);
props.Add("certificateapplicationpolicy", certificateApplicationPolicy);

entry.TryGetArrayProperty(LDAPProperties.CertificatePolicy, out var certificatePolicy);
Expand All @@ -477,7 +491,7 @@ public static Dictionary<string, object> ReadCertTemplateProperties(IDirectoryOb
}

entry.TryGetArrayProperty(LDAPProperties.ApplicationPolicies, out var appPolicies);

props.Add("applicationpolicies",
ParseCertTemplateApplicationPolicies(appPolicies,
schemaVersion, hasUseLegacyProvider));
Expand Down Expand Up @@ -520,6 +534,7 @@ public async Task<IssuancePolicyProperties> ReadIssuancePolicyProperties(IDirect
/// <param name="entry"></param>
public Dictionary<string, object> ParseAllProperties(IDirectoryObject entry) {
var props = new Dictionary<string, object>();

foreach (var property in entry.PropertyNames()) {
if (ReservedAttributes.Contains(property, StringComparer.OrdinalIgnoreCase))
continue;
Expand All @@ -530,13 +545,37 @@ public Dictionary<string, object> ParseAllProperties(IDirectoryObject entry) {

if (collCount == 1) {
var testString = entry.GetProperty(property);

if (!string.IsNullOrEmpty(testString))
if (!string.IsNullOrEmpty(testString)) {
if (property.Equals("badpasswordtime", StringComparison.OrdinalIgnoreCase))
props.Add(property, Helpers.ConvertFileTimeToUnixEpoch(testString));
else
props.Add(property, BestGuessConvert(testString));
}
} else {
if (entry.TryGetByteProperty(property, out var testBytes)) {
if (testBytes == null || testBytes.Length == 0) {
continue;
}

// SIDs
try {
var sid = new SecurityIdentifier(testBytes, 0);
props.Add(property, sid.Value);
continue;
} catch {
/* Ignore */
}

// GUIDs
try {
var guid = new Guid(testBytes);
props.Add(property, guid.ToString());
continue;
} catch {
/* Ignore */
}
}

if (entry.TryGetArrayProperty(property, out var arr) && arr.Length > 0) {
props.Add(property, arr.Select(BestGuessConvert).ToArray());
}
Expand Down Expand Up @@ -577,23 +616,28 @@ private static string[] ParseCertTemplateApplicationPolicies(string[] applicatio
/// <summary>
/// Does a best guess conversion of the property to a type useable by the UI
/// </summary>
/// <param name="property"></param>
/// <param name="value"></param>
/// <returns></returns>
private static object BestGuessConvert(string property) {
private static object BestGuessConvert(string value) {
//Parse boolean values
if (bool.TryParse(property, out var boolResult)) return boolResult;
if (bool.TryParse(value, out var boolResult)) return boolResult;

//A string ending with 0Z is likely a timestamp
if (property.EndsWith("0Z")) return Helpers.ConvertTimestampToUnixEpoch(property);
if (value.EndsWith("0Z")) return Helpers.ConvertTimestampToUnixEpoch(value);

//This string corresponds to the max int, and is usually set in accountexpires
if (property == "9223372036854775807") return -1;
if (value == "9223372036854775807") return -1;

//Try parsing as an int
if (int.TryParse(property, out var num)) return num;
if (int.TryParse(value, out var num)) return num;

// If we have binary unicode, encode it
foreach (char c in value) {
if (char.IsControl(c)) return System.Text.Encoding.UTF8.GetBytes(value);
}

//Just return the property as a string
return property;
return value;
}

/// <summary>
Expand Down
17 changes: 14 additions & 3 deletions test/unit/Facades/MockDirectoryObject.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,11 @@ public bool TryGetArrayProperty(string propertyName, out string[] value) {

var temp = Properties[propertyName];
if (temp.IsArray()) {
value = temp as string[];
if (temp is string[] s) {
value = s;
return true;
}
value = Array.Empty<string>();
return true;
}

Expand Down Expand Up @@ -153,8 +157,15 @@ public int PropertyCount(string propertyName) {
var property = Properties[propertyName];
if (property.IsArray())
{
var cast = property as string[];
return cast?.Length ?? 0;
if (property is string[] s) {
return s.Length;
}

if (property is byte[] b) {
return b.Length;
}

return 0;
}

return 1;
Expand Down
62 changes: 60 additions & 2 deletions test/unit/LdapPropertyTests.cs
View file
Original file line number Diff line number Diff line change
@@ -1,15 +1,19 @@
using System;
using System;
using System.Collections.Generic;
using System.Security.Cryptography;
using System.Security.Cryptography.X509Certificates;
using System.Security.Principal;
using System.Threading.Tasks;
using CommonLibTest.Facades;
using SharpHoundCommonLib;
using SharpHoundCommonLib.Enums;
using SharpHoundCommonLib.OutputTypes;
using SharpHoundCommonLib.Processors;
using SharpHoundRPC;
using Xunit;
using Xunit.Abstractions;
using static System.Text.Encoding;

// ReSharper disable StringLiteralTypo

namespace CommonLibTest
Expand Down Expand Up @@ -855,7 +859,8 @@ public void LDAPPropertyProcessor_ParseAllProperties()
{"name", "NTAUTHCERTIFICATES@DUMPSTER.FIRE"},
{"domainsid", "S-1-5-21-2697957641-2271029196-387917394"},
{"whencreated", 1683986131},
}, "","2F9F3630-F46A-49BF-B186-6629994EBCF9");
{LDAPProperties.DSASignature, "jkr"}
}, "", "2F9F3630-F46A-49BF-B186-6629994EBCF9");

var processor = new LdapPropertyProcessor(new MockLdapUtils());
var props = processor.ParseAllProperties(mock);
Expand All @@ -865,6 +870,7 @@ public void LDAPPropertyProcessor_ParseAllProperties()
Assert.DoesNotContain("description", keys);
Assert.DoesNotContain("whencreated", keys);
Assert.DoesNotContain("name", keys);
Assert.DoesNotContain(LDAPProperties.DSASignature, keys);

Assert.Contains("domainsid", keys);
Assert.Contains("domain", keys);
Expand Down Expand Up @@ -929,5 +935,57 @@ public void LDAPPropertyProcessor_ParseAllProperties_CollectionCountOne_NotBadPa
Assert.Single(keys);
}

[Fact]
public void LDAPPropertyProcessor_ParseAllProperties_CollectionCountOne_ControlCharactersAreEncoded() {
var mock = new MockDirectoryObject("CN\u003dNTAUTHCERTIFICATES,CN\u003dPUBLIC KEY SERVICES,CN\u003dSERVICES,CN\u003dCONFIGURATION,DC\u003dDUMPSTER,DC\u003dFIRE",
new Dictionary<string, object>
{{"usercertificate", "\u0000"}}, "", "2F9F3630-F46A-49BF-B186-6629994EBCF9");

var processor = new LdapPropertyProcessor(new MockLdapUtils());
var props = processor.ParseAllProperties(mock);
var keys = props.Keys;

Assert.Contains("usercertificate", keys);
Assert.Single(keys);
var hasCert = props.TryGetValue("usercertificate", out var usercert);
Assert.True(hasCert);
Assert.Equal("\u0000", UTF8.GetString(usercert as byte[]));
}

[WindowsOnlyFact]
public void LDAPPropertyProcessor_ParseAllProperties_CollectionCountOne_SID() {
var creatorSIDExpected = "S-1-5-21-2697957641-2271029196-387917394";
var sidBytes = new SecurityIdentifier(creatorSIDExpected).GetBytes();

Check warning on line 958 in test/unit/LdapPropertyTests.cs

View workflow job for this annotation

GitHub Actions / build 

This call site is reachable on all platforms. 'SecurityIdentifier' is only supported on: 'windows'. (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1416)

Check warning on line 958 in test/unit/LdapPropertyTests.cs

View workflow job for this annotation

GitHub Actions / build 

This call site is reachable on all platforms. 'SecurityIdentifier' is only supported on: 'windows'. (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1416)
var mock = new MockDirectoryObject("CN\u003dNTAUTHCERTIFICATES,CN\u003dPUBLIC KEY SERVICES,CN\u003dSERVICES,CN\u003dCONFIGURATION,DC\u003dDUMPSTER,DC\u003dFIRE",
new Dictionary<string, object>
{{"ms-ds-creatorsid", sidBytes}}, "", "2F9F3630-F46A-49BF-B186-6629994EBCF9");

var processor = new LdapPropertyProcessor(new MockLdapUtils());
var props = processor.ParseAllProperties(mock);
var keys = props.Keys;

Assert.Contains("ms-ds-creatorsid", keys);
Assert.Single(keys);
var hasSID = props.TryGetValue("ms-ds-creatorsid", out var creatorSIDActual);
Assert.True(hasSID);
Assert.Equal(creatorSIDExpected, creatorSIDActual.ToString());
}

[Fact]
public void LDAPPropertyProcessor_ParseAllProperties_GUID() {
var guidExpected = Guid.NewGuid();
var mock = new MockDirectoryObject("CN\u003dNTAUTHCERTIFICATES,CN\u003dPUBLIC KEY SERVICES,CN\u003dSERVICES,CN\u003dCONFIGURATION,DC\u003dDUMPSTER,DC\u003dFIRE",
new Dictionary<string, object>
{{"guid", guidExpected.ToByteArray()}}, "", "2F9F3630-F46A-49BF-B186-6629994EBCF9");

var processor = new LdapPropertyProcessor(new MockLdapUtils());
var props = processor.ParseAllProperties(mock);
var keys = props.Keys;

Assert.Single(keys);
var hasGuid = props.TryGetValue("guid", out var guidActual);
Assert.True(hasGuid);
Assert.Equal(guidExpected.ToString(), guidActual);
}
}
}

0 comments on commit 15553c2

Please sign in to comment.