Drynooo - Malicious users can bypass the blacklist.

[sherlock-admin2]

Drynooo

high

Malicious users can bypass the blacklist.

Summary

The protocol sets the blacklist through roles, and users can bypass the blacklist through the renounceRole function.

Vulnerability Detail

mTBILL does not allow blacklisted users to transfer funds.

    function _beforeTokenTransfer(
        address from,
        address to,
        uint256 amount
    )
        internal
        virtual
        override(ERC20PausableUpgradeable)
        onlyNotBlacklisted(from)
        onlyNotBlacklisted(to)
    {
        ERC20PausableUpgradeable._beforeTokenTransfer(from, to, amount);
    }

But it is implemented in the form of giving BLACKLISTED_ROLE.

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

The AccessControlUpgradeable contract has a renounceRole function, through which users can give up their BLACKLISTED_ROLE, thereby bypassing the blacklist.

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

        _revokeRole(role, account);
    }

Impact

Malicious users can bypass the blacklist.

Code Snippet

    function _beforeTokenTransfer(
        address from,
        address to,
        uint256 amount
    )
        internal
        virtual
        override(ERC20PausableUpgradeable)
        onlyNotBlacklisted(from)
        onlyNotBlacklisted(to)
    {
        ERC20PausableUpgradeable._beforeTokenTransfer(from, to, amount);
    }

Tool used

Manual Review

Recommendation

It is recommended not to use roles to implement blacklists.

[pkqs90]

Nice issue. I missed the escalation period but still want to make a comment - should this therotically be medium instead of high, since it won't cause a direct loss of funds or non-material losses?

[amankakar]

Nice issue. I missed the escalation period but still want to make a comment - should this therotically be medium instead of high, since it won't cause a direct loss of funds or non-material losses?

Although i have missed this finding But I think the High is appropriate because the user can easily bypass the blacklist status.

[blutorque]

I think this is more severe, considering the mTBill can immediately be traded in a secondary market(such as Morpho), which already hold ~30% of mTBill supply.