sherlock-audit / 2023-02-kairos-judging

2 stars 0 forks source link

Koolex - Inconsistent handling of ERC20 tokens transferring #145

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

Koolex

medium

Inconsistent handling of ERC20 tokens transferring

Summary

Transferring ERC20 tokens which do not return a bool value will always revert

Vulnerability Detail

The protocol supports any ERC20 token, and use checkedTransferFrom method to check the returned bool value for non-reverting ERC20 tokens. However, Some ERC20 tokens do not return a bool value on any ERC20 operations (e.g. transfer). Those tokens will always revert.

Impact

Transferring ERC20 tokens which do not return a bool value will always revert

Code Snippet

  function checkedTransferFrom(IERC20 currency, address from, address to, uint256 amount) internal {
        if (!currency.transferFrom(from, to, amount)) {
            revert ERC20TransferFailed(currency, from, to);
        }
    }

    function checkedTransfer(IERC20 currency, address to, uint256 amount) internal {
        if (!currency.transfer(to, amount)) {
            revert ERC20TransferFailed(currency, address(this), to);
        }
    }

https://github.com/sherlock-audit/2023-02-kairos/blob/main/kairos-contracts/src/utils/Erc20CheckedTransfer.sol#L9-L18

Tool used

Manual Review

Recommendation

Use a safe transfer from a library like OpenZeppelin SafeERC20 to avoid inconsistent handling of ERC20 transfer

Duplicate of #1