`Governor.getUsersByRole` and `Governor.getRolesForUser` return 255 results at most

[c4-submissions]

Lines of code

https://github.com/code-423n4/2023-10-badger/blob/f2f2e2cf9965a1020661d179af46cb49e993cb7e/packages/contracts/contracts/Governor.sol#L73-L91 https://github.com/code-423n4/2023-10-badger/blob/f2f2e2cf9965a1020661d179af46cb49e993cb7e/packages/contracts/contracts/Governor.sol#L96-L115

Vulnerability details

Impact

According to comments, the contract can support up to 256 roles. But Governor.getRolesForUser and Governor.getRolesFromByteMap can only return 255 roles at most

Proof of Concept

In Governor.getRolesForUser, in the for-loop, i is between between [0, 255), which makes 255 results at most

    function getRolesForUser(address user) external view returns (uint8[] memory rolesForUser) {
        // Enumerate over all possible roles and check if enabled
        uint256 count;
        for (uint8 i = 0; i < type(uint8).max; i++) {  <<<-------- Here 255 result at most
            if (doesUserHaveRole(user, i)) {
                count += 1;
            }
        }
        if (count > 0) {
            uint256 j = 0;
            rolesForUser = new uint8[](count);
            for (uint8 i = 0; i < type(uint8).max; i++) { <<<-------- Here 255 result at most
                if (doesUserHaveRole(user, i)) {
                    rolesForUser[j] = i;
                    j++;
                }
            }
        }
    }

Similar code in Governor.getRolesFromByteMap

    function getRolesFromByteMap(bytes32 byteMap) public pure returns (uint8[] memory roleIds) {
        uint256 count;
        for (uint8 i = 0; i < type(uint8).max; i++) { <<<-------- Here 255 result at most
            bool roleEnabled = (uint256(byteMap >> i) & 1) != 0;
            if (roleEnabled) {
                count += 1;
            }
        }
        if (count > 0) {
            uint256 j = 0;
            roleIds = new uint8[](count);
            for (uint8 i = 0; i < type(uint8).max; i++) {
                bool roleEnabled = (uint256(byteMap >> i) & 1) != 0;
                if (roleEnabled) {
                    roleIds[j] = i;
                    j++;
                }
            }
        }
    }

Tools Used

VIM

Recommended Mitigation Steps

Assessed type

Loop

[bytes032]

• Spec bug
• Over inflated severity

[c4-pre-sort]

bytes032 marked the issue as primary issue

[c4-pre-sort]

bytes032 marked the issue as insufficient quality report

[c4-judge]

jhsagd76 changed the severity to QA (Quality Assurance)

[c4-judge]

jhsagd76 marked the issue as grade-a

[c4-judge]

jhsagd76 marked the issue as selected for report

[c4-judge]

jhsagd76 marked the issue as not selected for report