Skip to content

Commit

Permalink
fix: catch overflow exception for ACL processor properly
Browse files Browse the repository at this point in the history
chore: add success computer status messages
chore: update some log levels and messages
  • Loading branch information
rvazarkar committed Oct 10, 2022
1 parent 1c54180 commit 1ec2397
Show file tree
Hide file tree
Showing 7 changed files with 132 additions and 14 deletions.
1 change: 1 addition & 0 deletions src/CommonLib/CSVComputerStatus.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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; }
Expand Down
1 change: 1 addition & 0 deletions src/CommonLib/OutputTypes/Computer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand Down
26 changes: 22 additions & 4 deletions src/CommonLib/Processors/ACLProcessor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,17 @@ public IEnumerable<ACE> 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)));

Expand Down Expand Up @@ -416,13 +426,21 @@ public IEnumerable<ACE> 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)))
{
Expand Down
41 changes: 38 additions & 3 deletions src/CommonLib/Processors/ComputerAvailability.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -37,6 +39,8 @@ public ComputerAvailability(PortScanner scanner, int timeout = 500, int computer
_skipPasswordCheck = skipPasswordCheck;
}

public event ComputerStatusDelegate ComputerStatusEvent;

/// <summary>
/// Helper function to use commonlib types for IsComputerAvailable
/// </summary>
Expand Down Expand Up @@ -67,8 +71,14 @@ public async Task<ComputerStatus> 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,
Expand All @@ -83,9 +93,15 @@ public async Task<ComputerStatus> 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,
Expand All @@ -104,20 +120,39 @@ public async Task<ComputerStatus> 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,
Error = ComputerStatus.PortNotOpen
};
}

_log.LogDebug("{ComputerName} is available for enumeration", computerName);

SendComputerStatus(new CSVComputerStatus
{
Status = CSVComputerStatus.StatusSuccess,
Task = "ComputerAvailability",
ComputerName = computerName
});

return new ComputerStatus
{
Connectable = true,
Error = null
};
}

private void SendComputerStatus(CSVComputerStatus status)
{
ComputerStatusEvent?.Invoke(status);
}
}
}
50 changes: 45 additions & 5 deletions src/CommonLib/Processors/ComputerSessionProcessor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -47,12 +47,26 @@ public async Task<SessionAPIResult> 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<Session>();

Expand All @@ -67,17 +81,16 @@ public async Task<SessionAPIResult> 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;
}

Expand Down Expand Up @@ -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<TypedPrincipal>();
Expand Down Expand Up @@ -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,
Expand All @@ -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;
Expand Down
15 changes: 13 additions & 2 deletions src/CommonLib/Processors/LocalGroupProcessor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,8 @@ public IEnumerable<LocalGroupAPIResult> 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
{
Expand All @@ -130,6 +131,7 @@ public IEnumerable<LocalGroupAPIResult> GetLocalGroups(string computerName, stri
ret.Collected = false;
ret.FailureReason = $"SamOpenAliasInDomain failed with status {openAliasResult.Status}";
yield return ret;
continue;
}

var results = new List<TypedPrincipal>();
Expand All @@ -148,8 +150,16 @@ public IEnumerable<LocalGroupAPIResult> 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))
Expand Down Expand Up @@ -245,7 +255,8 @@ public IEnumerable<LocalGroupAPIResult> 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)
{
Expand Down
12 changes: 12 additions & 0 deletions src/CommonLib/Processors/UserRightsAssignmentProcessor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@ public IEnumerable<UserRightsAssignmentAPIResult> 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",
Expand All @@ -59,6 +61,9 @@ public IEnumerable<UserRightsAssignmentAPIResult> 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,
Expand All @@ -71,6 +76,13 @@ public IEnumerable<UserRightsAssignmentAPIResult> GetUserRightsAssignments(strin
continue;
}

SendComputerStatus(new CSVComputerStatus
{
ComputerName = computerName,
Status = CSVComputerStatus.StatusSuccess,
Task = "LSAEnumerateAccountsWithUserRight"
});

if (!Cache.GetMachineSid(computerDomainSid, out var machineSid))
{
var getMachineSidResult = server.GetLocalDomainInformation();
Expand Down

0 comments on commit 1ec2397

Please sign in to comment.