hats-finance / Euro-Dollar-0xa4ccd3b6daa763f729ad59eae75f9cbff7baf2cd

Audit competition repository for Euro-Dollar (0xa4ccd3b6daa763f729ad59eae75f9cbff7baf2cd)
https://hats.finance
MIT License
3 stars 2 forks source link

Role Bearers Can Renounce Their Roles, Potentially Disrupting Contract Functionality #26

Open hats-bug-reporter[bot] opened 2 weeks ago

hats-bug-reporter[bot] commented 2 weeks ago

Github username: -- Twitter username: -- Submission hash (on-chain): 0x0e9694d933c5fc7d9a215d29d46229f566dc5cd461e04729a8d3bf4679c7f5b8 Severity: high

Description: Description\ In the Validator contract, which inherits from OpenZeppelin's AccessControl, users assigned specific roles such as WHITELISTER_ROLE and BLACKLISTER_ROLE have the ability to renounce their roles by calling the renounceRole function. This function is provided by the AccessControl contract and allows any role bearer to relinquish their role

Issue:

Impact:

Recommendation:

To prevent unintended loss of critical roles and maintain the contract's functionality, consider overriding the renounceRole function to disable role renouncement. This approach has been utilized in other contracts within the same project (e.g., InvestToken and USDE contracts), where the renounceRole function is overridden to always revert:

Implementation Example:

function renounceRole(bytes32, address) public pure override {
    revert("Role renouncement is disabled");
}
AndreiMVP commented 2 weeks ago

renounceRole is not such a big risk if we already put a lot of trust in the wallets corresponding to the roles. It's a nice practice to prevent renouncing but not really a rule and while for token contracts it's been added because we have more roles, in the other contracts which are more specialized I'm fine with either having or not having it. Thus, not an issue.