AccessControlEnumerable doesn't override all internal functions

[hacker-DOM]

Background

AccessControlEnumerable inherits from AccessControl. It overrides some public methods, but not all internal ones. For example not this one

openzeppelin-contracts/contracts/access/AccessControl.sol

Lines 205 to 210 in ee14987

	function _grantRole(bytes32 role, address account) internal virtual {
	if (!hasRole(role, account)) {
	_roles[role].members[account] = true;
	emit RoleGranted(role, account, _msgSender());
	}
	}

Vulnerability scenario

Alice is writing a contract C. I've provided mocks for the two OZ contracts to illustrate the scenario:

contract AccessControl {
    mapping (bytes32 => mapping (address => bool)) _roles;
    function _grantRole(bytes32 role, address account) internal {
        _roles[role][account] = true;
    }
}

contract AccessControlEnumerable is AccessControl {
    
}

contract C is AccessControlEnumerable {
    constructor() {
        _grantRole(hex"deadbeef", msg.sender);
    }
}

C.getRoleMemberCount(hex"deadbeef") will return 0.

Recommendation

Short term, override the internal methods of AccessControlEnumerable. This will ensure the above code behaves as expected.

Long term, remove the dependency of AccessControlEnumerable on AccessControl. AddressSet has methods to get enumeration and inclusion, so there is no need for such redundancy.

[Amxx]

Hello @hacker-DOM

Thank for raising that issue. AccessControl was implemented when AccessControl's grant and revoke functions were private and not internal. Now that they are internal, AccessControl should be refactored accordingly