Blacklist functionality of MTBill can be bypassed

Summary

MTBill has a Blacklistable functionality that based on openzeppelin's AccessControlUpgradeable. Due to implementation and docs Blacklisted user should be assigned to BLACKLISTED_ROLE by BLACKLIST_OPERATOR_ROLE-admin, and should be removed by the admin as well. But this flow can be bypassed and user can remove himself from the blacklist.

Vulnerability Detail

AccessControlUpgradeable brings an ability for any address with a role to renounce it's own role by using renounceRole(). Such behaviour will break completely Blacklist protection for MTBill.

Attacker account have been moved to Blacklist: BLACKLISTED_ROLE - granted by admin.
Attacker account execute renounceRole(BLACKLISTED_ROLE, himself): BLACKLISTED_ROLE - revoked by attacker.
Attacker can continue to execute transactions with MTBill.

Impact

Blacklist functionality can be bypassed by account that leads to broken protection of MTBill and enables transactions for restricted account.

Code Snippet

https://github.com/sherlock-audit/2024-05-midas/blob/main/midas-contracts/contracts/access/Blacklistable.sol#L38-L41

    function _onlyNotBlacklisted(address account)
        private
        view
        onlyNotRole(BLACKLISTED_ROLE, account)
    {}

https://github.com/sherlock-audit/2024-05-midas/blob/main/midas-contracts/contracts/access/WithMidasAccessControl.sol#L64-L66

    function _onlyNotRole(bytes32 role, address account) internal view {
        require(!accessControl.hasRole(role, account), "WMAC: has role");
    }

https://github.com/sherlock-audit/2024-05-midas/blob/main/midas-contracts/contracts/access/MidasAccessControl.sol#L14-L17

contract MidasAccessControl is
    AccessControlUpgradeable,
    MidasInitializable,
    MidasAccessControlRoles

https://github.com/OpenZeppelin/openzeppelin-contracts-upgradeable/blob/v4.9.0/contracts/access/AccessControlUpgradeable.sol#L186-L190

    function renounceRole(bytes32 role, address account) public virtual override {
        require(account == _msgSender(), "AccessControl: can only renounce roles for self");

        _revokeRole(role, account);
    }

Tool used

Manual Review

Recommendation

Disable(at least for BLACKLISTED_ROLE) renounceRole() functionality by overriding it.

Duplicate of #9

sherlock-audit / 2024-05-midas-judging

deepkin - Blacklist functionality of MTBill can be bypassed #60