Skip to content

Fix Issues with ACE type filtering and ObjectType GUID parsing in BadSuccessor.py search_ous() that causes False negatives#2170

Merged
alexisbalbachan merged 3 commits into
fortra:masterfrom
ThatTotallyRealMyth:fix-badsuccessor-ace-parsing
May 5, 2026
Merged

Fix Issues with ACE type filtering and ObjectType GUID parsing in BadSuccessor.py search_ous() that causes False negatives#2170
alexisbalbachan merged 3 commits into
fortra:masterfrom
ThatTotallyRealMyth:fix-badsuccessor-ace-parsing

Conversation

@ThatTotallyRealMyth
Copy link
Copy Markdown
Contributor

this pr fixes two issues in the badsuccessor.py search_ous() function which is in charge of finding actually finding/detecting the badsuccessor vuln.

First, the ACE filtering logic was only processing standard allow ACEs and was not correctly handling object-specific allow ACEs. This meant relevant ACCESS_ALLOWED_OBJECT_ACE entries were discarded in the if block of search_ous(). Even tho these can cause the same vulnerability to arise and the original Akamai script successfully detects.

Second, object-specific ACE filtering by schema object type was not working and instead it just failed silently. This causes us to pick up identifies that we don't usually want(like print and account operators). Previously, the code attempted to read ObjectType via getattr(...) and compared the raw bytes value directly, which prevented valid schema GUIDs from being parsed and matched with what the code is actually looking for.

These issue is not present in the powershell implementation of the exploit as powershell uses .NET that handles this appropriately.

A vendor product in my clients environment created an OU that was vulnerable and the script was not able to detect it. I only found it by chance and upon trying the powershell version, I was able to confirm the vulnerability.

Additionally, I would like to open a separate PR to port badsuccesor and other scripts off ldap3 and to impackets ldap. I have faced in this engagement issues with tooling not working with ldap3 because of the fact that it doesn't support SASL with NTLM authentication when I want to connect to a DC with LDAP signing without a TLS cert present on LDAPS.

Comment thread examples/badsuccessor.py Outdated
@alexisbalbachan alexisbalbachan added the waiting for response Further information is needed from people who opened the issue or pull request label Apr 23, 2026
Co-authored-by: alexisbalbachan <alexisbalbachan@gmail.com>
@ThatTotallyRealMyth
Copy link
Copy Markdown
Contributor Author

Hey @alexisbalbachan I made sure to properly test in my lab and I appreciated your suggestion and flagging for me. Can I also ask, I made a PR for #2171, is there anything I can do to get it across the finish line in terms of tests or providing output running in different types of environments?

side note, I have been working on a module to add support for MS-NEGOEX but have struggled a bit with some stuff since the spec defines some new GSS-API functions that Im not sure how to handle. I read in current implementations that impacket doesn't do the full specs so I wasn't sure if that means that the optional meta data stuff or other elements could be skipped. Any chance I could get some help with code formatting/structure to get it ready by the time the next release roles around?

@anadrianmanrique
Copy link
Copy Markdown
Collaborator

@ThatTotallyRealMyth Thanks for submitting!
Regarding #2171 it will be reviewed in the context of 0.14 release. We're planning to have a intermediate 0.13.1 release in a couple of weeks, so we're focusing on merging the last changes before a little code freeze period.
Regarding MS-NEGOEX , you can submit a partial implementation as PR. We'll be happy to help with it for sure, in case you got stuck.
Thanks

@ThatTotallyRealMyth
Copy link
Copy Markdown
Contributor Author

Thank you very much @anadrianmanrique for the heads up!! I think the module is 70% ready albeit prolly has very small/minor bugs that I haven't been able to get over. I will polish it up with properly comments and docstrings notes before opening a PR.

I noticed the PR still said waiting for response and a red box saying 1 requested change. I thought I approved it already but do let me know if there was other information needed from my end.

@anadrianmanrique
Copy link
Copy Markdown
Collaborator

@ThatTotallyRealMyth I think PR should be ready to be merged. We will double check with @alexisbalbachan . No worries. Thanks!

@anadrianmanrique anadrianmanrique removed the waiting for response Further information is needed from people who opened the issue or pull request label May 1, 2026
@ThatTotallyRealMyth
Copy link
Copy Markdown
Contributor Author

@ThatTotallyRealMyth I think PR should be ready to be merged. We will double check with @alexisbalbachan . No worries. Thanks!

Thank you very much all heh and I appreciate the responsiveness :3

@alexisbalbachan
Copy link
Copy Markdown
Collaborator

Hey @ThatTotallyRealMyth, thank you for your work, i'm merging this PR now

@alexisbalbachan alexisbalbachan merged commit 8601e43 into fortra:master May 5, 2026
7 checks passed
@ThatTotallyRealMyth ThatTotallyRealMyth deleted the fix-badsuccessor-ace-parsing branch May 6, 2026 10:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Unexpected problem or unintended behavior

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants