From 1ec23974dfd0e1e870a90f184bb44a56b2e785d5 Mon Sep 17 00:00:00 2001 From: Rohan Vazarkar Date: Mon, 10 Oct 2022 11:41:34 -0400 Subject: [PATCH] fix: catch overflow exception for ACL processor properly chore: add success computer status messages chore: update some log levels and messages --- src/CommonLib/CSVComputerStatus.cs | 1 + src/CommonLib/OutputTypes/Computer.cs | 1 + src/CommonLib/Processors/ACLProcessor.cs | 26 ++++++++-- .../Processors/ComputerAvailability.cs | 41 +++++++++++++-- .../Processors/ComputerSessionProcessor.cs | 50 +++++++++++++++++-- .../Processors/LocalGroupProcessor.cs | 15 +++++- .../UserRightsAssignmentProcessor.cs | 12 +++++ 7 files changed, 132 insertions(+), 14 deletions(-) diff --git a/src/CommonLib/CSVComputerStatus.cs b/src/CommonLib/CSVComputerStatus.cs index 9b075e34..faf34c19 100644 --- a/src/CommonLib/CSVComputerStatus.cs +++ b/src/CommonLib/CSVComputerStatus.cs @@ -5,6 +5,7 @@ namespace SharpHoundCommonLib { public class CSVComputerStatus { + public const string StatusSuccess = "Success"; public string ComputerName { get; set; } public string Task { get; set; } public string Status { get; set; } diff --git a/src/CommonLib/OutputTypes/Computer.cs b/src/CommonLib/OutputTypes/Computer.cs index d53dbb75..05cee05c 100644 --- a/src/CommonLib/OutputTypes/Computer.cs +++ b/src/CommonLib/OutputTypes/Computer.cs @@ -27,6 +27,7 @@ public class ComputerStatus public static string NonWindowsOS => "NonWindowsOS"; public static string OldPwd => "PwdLastSetOutOfRange"; public static string PortNotOpen => "PortNotOpen"; + public static string Success => "Success"; public CSVComputerStatus GetCSVStatus(string computerName) { diff --git a/src/CommonLib/Processors/ACLProcessor.cs b/src/CommonLib/Processors/ACLProcessor.cs index cf4605d5..fbdc9b0b 100644 --- a/src/CommonLib/Processors/ACLProcessor.cs +++ b/src/CommonLib/Processors/ACLProcessor.cs @@ -144,7 +144,17 @@ public IEnumerable ProcessACL(byte[] ntSecurityDescriptor, string objectDom } var descriptor = _utils.MakeSecurityDescriptor(); - descriptor.SetSecurityDescriptorBinaryForm(ntSecurityDescriptor); + try + { + descriptor.SetSecurityDescriptorBinaryForm(ntSecurityDescriptor); + } + catch (OverflowException) + { + _log.LogWarning( + "Security descriptor on object {Name} exceeds maximum allowable length. Unable to process", + objectName); + yield break; + } var ownerSid = PreProcessSID(descriptor.GetOwner(typeof(SecurityIdentifier))); @@ -416,13 +426,21 @@ public IEnumerable ProcessGMSAReaders(byte[] groupMSAMembership, string obj { if (groupMSAMembership == null) { - _log.LogTrace("GMSA bytes are null for {Name}", objectName); + _log.LogDebug("GMSA bytes are null for {Name}", objectName); yield break; } - var descriptor = _utils.MakeSecurityDescriptor(); - descriptor.SetSecurityDescriptorBinaryForm(groupMSAMembership); + try + { + descriptor.SetSecurityDescriptorBinaryForm(groupMSAMembership); + } + catch (OverflowException) + { + _log.LogWarning("GMSA ACL length on object {Name} exceeds allowable length. Unable to process", + objectName); + } + foreach (var ace in descriptor.GetAccessRules(true, true, typeof(SecurityIdentifier))) { diff --git a/src/CommonLib/Processors/ComputerAvailability.cs b/src/CommonLib/Processors/ComputerAvailability.cs index d2f14c0b..1222fb98 100644 --- a/src/CommonLib/Processors/ComputerAvailability.cs +++ b/src/CommonLib/Processors/ComputerAvailability.cs @@ -7,6 +7,8 @@ namespace SharpHoundCommonLib.Processors { public class ComputerAvailability { + public delegate void ComputerStatusDelegate(CSVComputerStatus status); + private readonly int _computerExpiryDays; private readonly ILogger _log; private readonly PortScanner _scanner; @@ -37,6 +39,8 @@ public ComputerAvailability(PortScanner scanner, int timeout = 500, int computer _skipPasswordCheck = skipPasswordCheck; } + public event ComputerStatusDelegate ComputerStatusEvent; + /// /// Helper function to use commonlib types for IsComputerAvailable /// @@ -67,8 +71,14 @@ public async Task IsComputerAvailable(string computerName, strin { if (operatingSystem != null && !operatingSystem.StartsWith("Windows", StringComparison.OrdinalIgnoreCase)) { - _log.LogTrace("{ComputerName} is not available because operating system {OperatingSystem} is not valid", + _log.LogDebug("{ComputerName} is not available because operating system {OperatingSystem} is not valid", computerName, operatingSystem); + SendComputerStatus(new CSVComputerStatus + { + Status = ComputerStatus.NonWindowsOS, + Task = "ComputerAvailability", + ComputerName = computerName + }); return new ComputerStatus { Connectable = false, @@ -83,9 +93,15 @@ public async Task IsComputerAvailable(string computerName, strin if (passwordLastSet < threshold) { - _log.LogTrace( + _log.LogDebug( "{ComputerName} is not available because password last set {PwdLastSet} is out of range", computerName, passwordLastSet); + SendComputerStatus(new CSVComputerStatus + { + Status = ComputerStatus.OldPwd, + Task = "ComputerAvailability", + ComputerName = computerName + }); return new ComputerStatus { Connectable = false, @@ -104,7 +120,13 @@ public async Task IsComputerAvailable(string computerName, strin if (!await _scanner.CheckPort(computerName, timeout: _scanTimeout)) { - _log.LogTrace("{ComputerName} is not available because port 445 is unavailable", computerName); + _log.LogDebug("{ComputerName} is not available because port 445 is unavailable", computerName); + SendComputerStatus(new CSVComputerStatus + { + Status = ComputerStatus.PortNotOpen, + Task = "ComputerAvailability", + ComputerName = computerName + }); return new ComputerStatus { Connectable = false, @@ -112,6 +134,14 @@ public async Task IsComputerAvailable(string computerName, strin }; } + _log.LogDebug("{ComputerName} is available for enumeration", computerName); + + SendComputerStatus(new CSVComputerStatus + { + Status = CSVComputerStatus.StatusSuccess, + Task = "ComputerAvailability", + ComputerName = computerName + }); return new ComputerStatus { @@ -119,5 +149,10 @@ public async Task IsComputerAvailable(string computerName, strin Error = null }; } + + private void SendComputerStatus(CSVComputerStatus status) + { + ComputerStatusEvent?.Invoke(status); + } } } \ No newline at end of file diff --git a/src/CommonLib/Processors/ComputerSessionProcessor.cs b/src/CommonLib/Processors/ComputerSessionProcessor.cs index 952558a7..ed96685b 100644 --- a/src/CommonLib/Processors/ComputerSessionProcessor.cs +++ b/src/CommonLib/Processors/ComputerSessionProcessor.cs @@ -47,12 +47,26 @@ public async Task ReadUserSessions(string computerName, string var result = _nativeMethods.NetSessionEnum(computerName); if (result.IsFailed) { + SendComputerStatus(new CSVComputerStatus + { + Status = result.Status.ToString(), + Task = "NetSessionEnum", + ComputerName = computerName + }); _log.LogDebug("NetSessionEnum failed on {ComputerName}: {Status}", computerName, result.Status); ret.Collected = false; ret.FailureReason = result.Status.ToString(); return ret; } + _log.LogDebug("NetSessionEnum succeeded on {ComputerName}", computerName); + SendComputerStatus(new CSVComputerStatus + { + Status = CSVComputerStatus.StatusSuccess, + Task = "NetSessionEnum", + ComputerName = computerName + }); + ret.Collected = true; var results = new List(); @@ -67,17 +81,16 @@ public async Task ReadUserSessions(string computerName, string //Filter out blank/null cnames/usernames if (string.IsNullOrWhiteSpace(computerSessionName) || string.IsNullOrWhiteSpace(username)) { - _log.LogTrace("Skipping session entry with null session/user"); + _log.LogTrace("Skipping NetSessionEnum entry with null session/user"); continue; } - //Filter out blank usernames, computer accounts, the user we're doing enumeration with, and anonymous logons if (username.EndsWith("$") || username.Equals(_currentUserName, StringComparison.CurrentCultureIgnoreCase) || username.Equals("anonymous logon", StringComparison.CurrentCultureIgnoreCase)) { - _log.LogTrace("Skipping session for {Username}", username); + _log.LogTrace("Skipping NetSessionEnum entry for {Username}", username); continue; } @@ -139,12 +152,26 @@ public SessionAPIResult ReadUserSessionsPrivileged(string computerName, if (result.IsFailed) { - _log.LogTrace("NetWkstaUserEnum failed on {ComputerName}: {Status}", computerName, result.Status); + SendComputerStatus(new CSVComputerStatus + { + Status = result.Status.ToString(), + Task = "NetWkstaUserEnum", + ComputerName = computerName + }); + _log.LogDebug("NetWkstaUserEnum failed on {ComputerName}: {Status}", computerName, result.Status); ret.Collected = false; ret.FailureReason = result.Status.ToString(); return ret; } + _log.LogDebug("NetWkstaUserEnum succeeded on {ComputerName}", computerName); + SendComputerStatus(new CSVComputerStatus + { + Status = result.Status.ToString(), + Task = "NetWkstaUserEnum", + ComputerName = computerName + }); + ret.Collected = true; var results = new List(); @@ -212,6 +239,13 @@ public SessionAPIResult ReadUserSessionsRegistry(string computerName, string com { key = RegistryKey.OpenRemoteBaseKey(RegistryHive.Users, computerName); ret.Collected = true; + SendComputerStatus(new CSVComputerStatus + { + Status = CSVComputerStatus.StatusSuccess, + Task = "RegistrySessionEnum", + ComputerName = computerName + }); + _log.LogDebug("Registry session enum succeeded on {ComputerName}", computerName); ret.Results = key.GetSubKeyNames().Where(subkey => SidRegex.IsMatch(subkey)).Select(x => new Session { ComputerSID = computerSid, @@ -222,7 +256,13 @@ public SessionAPIResult ReadUserSessionsRegistry(string computerName, string com } catch (Exception e) { - _log.LogTrace("Failed to open remote registry on {ComputerName}: {Status}", computerName, e.Message); + _log.LogDebug("Registry session enum failed on {ComputerName}: {Status}", computerName, e.Message); + SendComputerStatus(new CSVComputerStatus + { + Status = e.Message, + Task = "RegistrySessionEnum", + ComputerName = computerName + }); ret.Collected = false; ret.FailureReason = e.Message; return ret; diff --git a/src/CommonLib/Processors/LocalGroupProcessor.cs b/src/CommonLib/Processors/LocalGroupProcessor.cs index be0ec9f5..f4230dca 100644 --- a/src/CommonLib/Processors/LocalGroupProcessor.cs +++ b/src/CommonLib/Processors/LocalGroupProcessor.cs @@ -111,7 +111,8 @@ public IEnumerable GetLocalGroups(string computerName, stri foreach (var alias in getAliasesResult.Value) { - var resolvedName = ResolveGroupName(alias.Name, computerName, machineSid, computerDomain, alias.Rid, isDc, + var resolvedName = ResolveGroupName(alias.Name, computerName, machineSid, computerDomain, alias.Rid, + isDc, domainResult.Name.Equals("builtin", StringComparison.OrdinalIgnoreCase)); var ret = new LocalGroupAPIResult { @@ -130,6 +131,7 @@ public IEnumerable GetLocalGroups(string computerName, stri ret.Collected = false; ret.FailureReason = $"SamOpenAliasInDomain failed with status {openAliasResult.Status}"; yield return ret; + continue; } var results = new List(); @@ -148,8 +150,16 @@ public IEnumerable GetLocalGroups(string computerName, stri ret.Collected = false; ret.FailureReason = $"SamGetMembersInAlias failed with status {getMembersResult.Status}"; yield return ret; + continue; } + SendComputerStatus(new CSVComputerStatus + { + Task = $"GetMembersInAlias - {alias.Name}", + ComputerName = computerName, + Status = CSVComputerStatus.StatusSuccess + }); + foreach (var securityIdentifier in getMembersResult.Value) { if (IsSidFiltered(securityIdentifier)) @@ -245,7 +255,8 @@ public IEnumerable GetLocalGroups(string computerName, stri } } - private NamedPrincipal ResolveGroupName(string baseName, string computerName, string machineSid, string domainName, int groupRid, bool isDc, bool isBuiltIn) + private NamedPrincipal ResolveGroupName(string baseName, string computerName, string machineSid, + string domainName, int groupRid, bool isDc, bool isBuiltIn) { if (isDc) { diff --git a/src/CommonLib/Processors/UserRightsAssignmentProcessor.cs b/src/CommonLib/Processors/UserRightsAssignmentProcessor.cs index 6fa1b45c..47400a80 100644 --- a/src/CommonLib/Processors/UserRightsAssignmentProcessor.cs +++ b/src/CommonLib/Processors/UserRightsAssignmentProcessor.cs @@ -37,6 +37,8 @@ public IEnumerable GetUserRightsAssignments(strin var policyOpenResult = LSAPolicy.OpenPolicy(computerName); if (policyOpenResult.IsFailed) { + _log.LogDebug("LSAOpenPolicy failed on {ComputerName} with status {Status}", computerName, + policyOpenResult.Status); SendComputerStatus(new CSVComputerStatus { Task = "LSAOpenPolicy", @@ -59,6 +61,9 @@ public IEnumerable GetUserRightsAssignments(strin var enumerateAccountsResult = server.GetResolvedPrincipalsWithPrivilege(privilege); if (enumerateAccountsResult.IsFailed) { + _log.LogDebug( + "LSAEnumerateAccountsWithUserRight failed on {ComputerName} with status {Status} for privilege {Privilege}", + computerName, policyOpenResult.Status, privilege); SendComputerStatus(new CSVComputerStatus { ComputerName = computerName, @@ -71,6 +76,13 @@ public IEnumerable GetUserRightsAssignments(strin continue; } + SendComputerStatus(new CSVComputerStatus + { + ComputerName = computerName, + Status = CSVComputerStatus.StatusSuccess, + Task = "LSAEnumerateAccountsWithUserRight" + }); + if (!Cache.GetMachineSid(computerDomainSid, out var machineSid)) { var getMachineSidResult = server.GetLocalDomainInformation();