BloodHoundAD / SharpHound2

The Old BloodHound C# Ingestor (Deprecated)
509 stars 113 forks source link

Fix removal of non-computer collection methods when ComputerFile is used #52

Closed cnotin closed 5 years ago

cnotin commented 5 years ago

The binary operator used is incorrect. The intent is to remove the listed collection methods from resolved but by using the XOR ^ operator it instead adds these methods (if not already present). Proof:

>SharpHound.exe -c computeronly --computerfile foo.txt
Initializing BloodHound at 11:18 on 28/01/2019
Increasing ping timeout to 1 second for ComputerFile mode
ComputerFile detected! Removing non-computer collection methods
Resolved Collection Methods to Group, LocalGroup, GPOLocalGroup, Session, Trusts, ACL, Container, RDP, ObjectProps, DCOM

We see here that several non-computer collection methods were added instead of removed.

The patch replaces XOR by NAND & ~ operator. After the patch:

>SharpHound.exe -c computeronly --computerfile foo.txt
Initializing BloodHound at 13:09 on 28/01/2019
Increasing ping timeout to 1 second for ComputerFile mode
ComputerFile detected! Removing non-computer collection methods
Resolved Collection Methods to LocalGroup, Session, DCOM

Just to be sure that the removal works well, we can try by specifying all methods:

>SharpHound.exe -c all --computerfile foo.txt
Initializing BloodHound at 13:11 on 28/01/2019
Increasing ping timeout to 1 second for ComputerFile mode
ComputerFile detected! Removing non-computer collection methods
Resolved Collection Methods to LocalGroup, Session, DCOM

More info: https://en.wikipedia.org/wiki/Bit_manipulation#Bit_manipulation_in_the_C_programming_language XOR allows to toggle a bit, but NAND should be used to clear it.

cnotin commented 5 years ago

Note that there are other XOR in lines below my patch. I wasn't sure of the intent (no comment/print) so I left them as-is. You might want to fix those also.

rvazarkar commented 5 years ago

Yeah, I'm not a smart man. I'll merge this in.

cnotin commented 5 years ago

Bitwise operations are tricky ;) I let you check other cases (lines below) that might have the same problem: I wasn't sure of the intent so I left them.