code-423n4 / 2022-04-backd-findings

6 stars 4 forks source link

RoleManager.sol: `_revokeRole` doesn't remove from the _roleMembers[role] set #158

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-04-backd/blob/c856714a50437cb33240a5964b63687c9876275b/backd/contracts/access/RoleManager.sol#L155

Vulnerability details

Impact

In the file RoleManager.sol the function _revokeRole doesn't remove the account from the _roleMembers[role] set.

This makes getRoleMemberCount wrong (for the else part) therefore the renounceGovernance require, on the number of governor, useless and risky as there could be no governance anymore. This imply among other things, that no account would be admin of all other roles, as the Governance Role is admin to all others. So if the last governor is removed, access control would be frozen for the whole the protocol

Proof of Concept

(1) https://github.com/code-423n4/2022-04-backd/blob/c856714a50437cb33240a5964b63687c9876275b/backd/contracts/access/RoleManager.sol#L155

Tools Used

Hardhat-ts testing

Recommended Mitigation Steps

Remove from the set the corresponding account for the role

chase-manning commented 2 years ago

Duplicate of #164