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

1 stars 0 forks source link

Gravity does not support ERC20 tokens with built-in fee #138

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#L600-L608

Vulnerability details

Impact

Contract Gravity.sol does not properly handle ERC20 tokens that charge fee on their transfers. Implementation of such a tokens does not transfer exact amount provided to transfer() but part of it is charged as a fee, burned or used in some other way. This leads to incorrect accounting and effectively to loss of funds.

Proof of Concept

Tools Used

Manual Review / VSCode

Recommended Mitigation Steps

It is recommended to add support for ERC20 tokens with built-in fees. Example of the implementation:

    uint256 ourStartingBalance = IERC20(_tokenContract).balanceOf(address(this));
    IERC20(_tokenContract).safeTransferFrom(msg.sender, address(this), _amount);
    uint256 ourEndingBalance = IERC20(_tokenContract).balanceOf(address(this));

    require(ourEndingBalance > ourStartingBalance, "ERC20 reduced balance");

    state_lastEventNonce = state_lastEventNonce + 1;

    emit SendToCosmosEvent(
        _tokenContract,
        msg.sender,
        _destination,
        _amount,
        ourEndingBalance - ourStartingBalance,
        state_lastEventNonce
    );
mlukanova commented 2 years ago

Duplicate of #3