Include Membership property set in AddSelf and AddMember edges

[JimSycurity]

Description

Includes the Membership property set as an additional object ACE guid for AddSelf and AddMember edges.

Motivation and Context

This change adds additional coverage for ACL abuse related to changing the membership of group nodes by introducing the Membership property set ACEGuid to the edges.

Testing performed while researching AllValidatedWrites ACEs confirmed a gap in coverage for this property set:

This diagram shows how the Membership property set fits into the possible ACE options that allow for modifying the member property of a group:

Fixes BED-7069

How Has This Been Tested?

Created a matrix test in CORP1 domain of my lab using this New-MembershipTest.ps1 script. This creates a group object for each possible ACE type that can modify group membership. Prior to this change 9 of the 12 ACEs were created by SharpHound. After implementing this change, all 12 ACEs were covered.

This change is limited to only 2 files and 3 lines of code in SharpHoundCommon. The functionality works as expected with no additional changes to SharpHound, SharpHoundEnterprise, BloodHound, or BloodHoundEnterprise.

Screenshots (if appropriate):

BloodHound edges prior to change:

BloodHound edges after SHC change:

Types of changes

• Chore (a change that does not modify the application functionality)
• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

• Documentation updates are needed, and have been made accordingly.
• [] I have added and/or updated tests to cover my changes.
• All new and existing tests passed.
• My changes include a database migration.

Summary by CodeRabbit

Release Notes

• New Features
  • Enhanced access control handling for group membership operations with improved support for membership property management.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Walkthrough

A new constant MembershipPropertySet was added to the ACEGuids class, and two conditional checks in ACLProcessor were expanded to include this new GUID in group-related ACE type matching.

Changes

Cohort / File(s)	Summary
GUID Constant Addition src/CommonLib/Processors/ACEGuids.cs	Added new public constant string MembershipPropertySet with GUID value "bc0ac240-79a9-11d0-9020-00c04fc2d4cf" for membership property set reference.
ACE Type Matching Updates src/CommonLib/Processors/ACLProcessor.cs	Expanded two conditional checks to include MembershipPropertySet alongside WriteMember in group-related ACE handling: one in Self-rights branch for Group objects, and one in AddMember edge generation within GenericWrite handling.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~5–10 minutes

• Verify the GUID value is correct and corresponds to the intended membership property set
• Confirm the conditional logic placements in ACLProcessor are semantically appropriate for their respective ACE handling contexts
• Ensure no unintended side effects from treating MembershipPropertySet equivalently to WriteMember in both branches

Poem

🐰 A new GUID hops into the fold,

MembershipPropertySet, shiny and bold!

ACLProcessor checks twice with glee,

Group permissions dance, now wild and free! ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title directly and clearly describes the main change: adding the Membership property set to AddSelf and AddMember edges.
Description check	✅ Passed	The PR description provides comprehensive detail: clear description of changes, detailed motivation with technical context and issue link, thorough testing methodology with quantified results, and supporting diagrams/screenshots.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch jsykora/all-validated-writes

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}