search

code-423n4 / 2021-08-realitycards-findings

1 stars 0 forks source link

RCTreasury: new hasRole() function with string role #45

Open code423n4 opened 3 years ago

code423n4 commented 3 years ago

Handle

hickuphh3

Vulnerability details

Impact

It might be operationally easier (eg. reading permissions via etherscan, saves a few seconds having to search for the bytes32 constant and copy its value) to take in the role of type string instead of bytes32. It also has an added benefit of overloading the hasRole() function, where overriding was desired.

Recommended Mitigation Steps

Perhaps add it as an additional function to avoid having to change all treasury.checkPermissions() function calls (since the role input has to be modified too).

// TODO: add to interface
function hasRole(string memory role, address account)
    external
    view
    override
    returns (bool)
{
    bytes32 _role = keccak256(abi.encodePacked(role));
    return AccessControl.hasRole(_role, account);
}
Splidge commented 3 years ago

Unfortunately OpenZeppelin didn't make hasRole virtual so it can't be overridden, which is why I can't specify it directly in the interface and am forced to create such a useless checkPermissions function. However I can see the benefit of having this function so I'll add it in there but have to name it something else.

Splidge commented 3 years ago

Implemented here