Bug: WriteProperty/Self permission checks are unreachable in FilterDelegation due to incorrect condition nesting

[ystobiasjang]

Description

In HealthcheckAnalyzer.cs, the FilterDelegation method contains dead code where several important AD permission checks (WRITE_PROP_MEMBER, WRITE_PROP_GPLINK, WRITE_PROP_GPC_FILE_SYS_PATH, WRITE_PROPSET_MEMBERSHIP, VAL_WRITE_SELF_MEMBERSHIP) are defined but never executed due to an incorrect conditional nesting.

Affected File

• File: Healthcheck/HealthcheckAnalyzer.cs
• Function: FilterDelegation() (line 1705)
• Bug Location: lines 1871-1913

Root Cause

At line 1871, the following condition wraps the Self, WriteProperty, and ReadProperty checks:

if (((accessrule.ActiveDirectoryRights & ActiveDirectoryRights.ExtendedRight) != 0
    || ((int)accessrule.ActiveDirectoryRights & 4096) != 0)
    && accessrule.ObjectType == EnrollPermission)  // ← BUG: EnrollPermission check
{
    // ADS_RIGHT_DS_SELF (lines 1874-1883)
    // ADS_RIGHT_DS_WRITE_PROP (lines 1885-1901)
    // ADS_RIGHT_DS_READ_PROP (lines 1903-1912)
}

Inside this block, the code checks if accessrule.ObjectType matches GUIDs defined in:

Variable	Line	Rights
GuidsControlValidatedWrites	1645-1647	WRITE_PROPSET_MEMBERSHIP
GuidsControlProperties	1649-1653	WRITE_PROP_MEMBER, WRITE_PROP_GPLINK, WRITE_PROP_GPC_FILE_SYS_PATH
GuidsControlPropertiesSets	1657-1659	VAL_WRITE_SELF_MEMBERSHIP

The problem: The outer condition at line 1871 requires accessrule.ObjectType == EnrollPermission (GUID: 0e10c968-78fb-11d2-90d4-00c04f79dc55, defined at line 1661).

However, the inner checks compare accessrule.ObjectType against completely different GUIDs:

Right	GUID
WRITE_PROP_MEMBER	bf9679c0-0de6-11d0-a285-00aa003049e2
WRITE_PROP_GPLINK	f30e3bbe-9ff0-11d1-b603-0000f80367c1
WRITE_PROP_GPC_FILE_SYS_PATH	f30e3bc1-9ff0-11d0-b603-0000f80367c1
WRITE_PROPSET_MEMBERSHIP	bc0ac240-79a9-11d0-9020-00c04fc2d4cf
VAL_WRITE_SELF_MEMBERSHIP	bf9679c0-0de6-11d0-a285-00aa003049e2

Since accessrule.ObjectType cannot simultaneously equal EnrollPermission AND any of these other GUIDs, these checks are logically unreachable (dead code).

Impact

The following delegation rights are never detected even when they exist in AD security descriptors:

• WRITE_PROP_MEMBER - Write member attribute (group membership modification)
• WRITE_PROP_GPLINK - Write gpLink attribute (GPO linking)
• WRITE_PROP_GPC_FILE_SYS_PATH - Write GPC file system path
• WRITE_PROPSET_MEMBERSHIP - Write membership property set
• VAL_WRITE_SELF_MEMBERSHIP - Validated write to membership

These are security-sensitive permissions that should be reported in delegation analysis.

Suggested Fix

Move the Self, WriteProperty, and ReadProperty checks outside the EnrollPermission condition (lines 1871-1913 should be restructured):

else if ((accessrule.ObjectFlags & ObjectAceFlags.ObjectAceTypePresent) == ObjectAceFlags.ObjectAceTypePresent)
{
    // ADS_RIGHT_DS_CONTROL_ACCESS - ExtendedRight checks (existing, works correctly)
    if ((accessrule.ActiveDirectoryRights & ActiveDirectoryRights.ExtendedRight) == ActiveDirectoryRights.ExtendedRight)
    {
        foreach (var extendedright in GuidsControlExtendedRights) { ... }
    }

    // ADS_RIGHT_DS_SELF - Should be outside EnrollPermission condition
    if ((accessrule.ActiveDirectoryRights & ActiveDirectoryRights.Self) == ActiveDirectoryRights.Self)
    {
        foreach (var validatewrite in GuidsControlValidatedWrites)
        {
            if (validatewrite.Key == accessrule.ObjectType) { ... }
        }
    }

    // ADS_RIGHT_DS_WRITE_PROP - Should be outside EnrollPermission condition
    if ((accessrule.ActiveDirectoryRights & ActiveDirectoryRights.WriteProperty) == ActiveDirectoryRights.WriteProperty)
    {
        foreach (var controlproperty in GuidsControlProperties)
        {
            if (controlproperty.Key == accessrule.ObjectType) { ... }
        }
        foreach (var controlpropertyset in GuidsControlPropertiesSets)
        {
            if (controlpropertyset.Key == accessrule.ObjectType) { ... }
        }
    }

    // ADS_RIGHT_DS_READ_PROP - Should be outside EnrollPermission condition
    if ((accessrule.ActiveDirectoryRights & ActiveDirectoryRights.ReadProperty) == ActiveDirectoryRights.ReadProperty)
    {
        foreach (var controlproperty in ReadGuidsControlProperties)
        {
            if (controlproperty.Key == accessrule.ObjectType) { ... }
        }
    }
}

How to Reproduce

1. Create an ACE in AD with WriteProperty right on member attribute (GUID: bf9679c0-0de6-11d0-a285-00aa003049e2)
2. Run PingCastle healthcheck
3. Observe that WRITE_PROP_MEMBER is not reported in the Delegations output

Version

Confirmed on master branch. (2e44462)

[ystobiasjang]

This issue was written with the assistance of Claude Opus 4.5 and personally reviewed by me.

TL;DR

Bug (Dead Code): Lines 1871-1913 in FilterDelegation() are unreachable.

• The Self, WriteProperty, and ReadProperty checks are nested inside accessrule.ObjectType == EnrollPermission condition,
• but they compare ObjectType against different GUIDs (e.g., WRITE_PROP_MEMBER), which can never match EnrollPermission.
• These permission checks never execute.