code-423n4 / 2022-02-jpyc-findings

1 stars 0 forks source link

QA Report #62

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Missing Modifier on burn Function in FiatTokenV2.sol

When a burn is performed, conceptually what is happening is that tokens are being transferred to address(0). Therefore, according to the documentation supplied with this finding, the checkWhitelist(msg.sender, _amount) should be applied as it is in mint.

Additionally, the emit Transfer(msg.sender, address(0), _amount); call indicates to external tools that a transfer has taken place. An external tool might assume that msg.sender has been whitelisted for 100000+ transfers based on the fact that an event was emitted that suggests they have this capability.

Code in question:

    function burn(uint256 _amount)
        external
        whenNotPaused
        onlyMinters
        notBlocklisted(msg.sender)//@audit Check whitelist here since this is technically emittings a Transfer event to address(0)
    {
        uint256 balance = balances[msg.sender];
        require(_amount > 0, "FiatToken: burn amount not greater than 0");
        require(balance >= _amount, "FiatToken: burn amount exceeds balance");

        totalSupply_ = totalSupply_ - _amount;
        balances[msg.sender] = balance - _amount;
        emit Burn(msg.sender, _amount);
        emit Transfer(msg.sender, address(0), _amount);
    }

Informational - Note on 100_000+ Token Transfers

I'm sure this has been thought off, but an attacker desiring to send more than 100_000 tokens can simply send two transactions of 50_000 in its stead. If you wish to prevent large transfers of money without approval, perhaps it would make sense to keep track of how much is being transferred by a specific address within a certain timeframe. For example, each address can transfer a max of 100_000 per day or something of the like.

thurendous commented 2 years ago

Missing Modifier on burn Function in FiatTokenV2.sol Informational - Note on 100_000+ Token Transfers

Thanks! These are the issues we are aware of but it is unnecessary to make any change yet. We designed this function in purpose.