code-423n4 / 2022-05-cudos-findings

1 stars 0 forks source link

Tokens with fee on transfer are not supported #98

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-05-cudos/blob/de39cf3cd1f1e1cf211819b06d4acf6a043acda0/solidity/contracts/Gravity.sol#L595-L609

Vulnerability details

There are ERC20 tokens that charge fee for every transfer() or transferFrom().

In the current implementation, sendToCosmos() assumes that the received amount is the same as the transfer amount, and uses it to emit SendToCosmosEvent event.

As a result, when bridging the token back from Cosmos, it may revert because of insufficient balance.

https://github.com/code-423n4/2022-05-cudos/blob/de39cf3cd1f1e1cf211819b06d4acf6a043acda0/solidity/contracts/Gravity.sol#L595-L609

    function sendToCosmos(
        address _tokenContract,
        bytes32 _destination,
        uint256 _amount
    ) public nonReentrant  {
        IERC20(_tokenContract).safeTransferFrom(msg.sender, address(this), _amount);
        state_lastEventNonce = state_lastEventNonce.add(1);
        emit SendToCosmosEvent(
            _tokenContract,
            msg.sender,
            _destination,
            _amount,
            state_lastEventNonce
        );
    }

Recommendation

Consider comparing before and after balance to get the actual transferred amount:

function sendToCosmos(
    address _tokenContract,
    bytes32 _destination,
    uint256 _amount
) public nonReentrant {
    uint256 preBalance = IERC20(_tokenContract).balanceOf(address(this));
    IERC20(_tokenContract).safeTransferFrom(msg.sender, address(this), _amount);
    uint256 postBalance = IERC20(_tokenContract).balanceOf(address(this));

    state_lastEventNonce = state_lastEventNonce.add(1);
    emit SendToCosmosEvent(
        _tokenContract,
        msg.sender,
        _destination,
        postBalance.sub(preBalance),
        state_lastEventNonce
    );
}
mlukanova commented 2 years ago

Duplicate of #3