Admidio Missing Minimum Administrator Check in Role Membership Removal

Summary

Role::stopMembership() does not verify whether removing a user from the administrator role leaves zero administrators. The deprecated Membership::stopMembership() contains this safety check, but the current code path bypasses it. Any administrator can remove the last remaining other administrator, locking the entire system out of administrative access. The exploit does not require concurrent requests; sequential removals produce the same result.

Details

Role::stopMembership() in src/Roles/Entity/Role.php stops a user's membership in a role without verifying whether the action leaves the administrator role with zero members:

// src/Roles/Entity/Role.php - Role::stopMembership()
public function stopMembership(int $userId): bool
{
    // No check for minimum administrator count
    // Directly updates membership end date
}

The deprecated Membership::stopMembership() contains this safety check and raises SYS_MUST_HAVE_ADMINISTRATOR when the removal would leave no admins, but current code paths no longer call this method.

Role::setMembership() includes a guard that prevents a user from removing their own administrator membership:

if ($userId === $gCurrentUserId) {
    // Prevents self-removal from admin role
}

This guard does not prevent an administrator from removing the last other administrator. Consider a system with exactly two administrators (Admin A and Admin B):

1. Admin A removes Admin B from the administrator role. The self-removal check passes (Admin A is not removing themselves). No minimum-count check runs. Admin B loses admin access.
2. Admin A is now the sole administrator. Admin A cannot remove themselves (self-removal guard), but the system is one compromised account away from total lockout.
3. If Admin A and Admin B each send a removal request for the other (sequentially or concurrently), both succeed. The system has zero administrators.

The core bug is the missing minimum-administrator check in Role::stopMembership(), not timing. Sequential requests reproduce the issue just as concurrent ones do.

Proof of Concept