Usage of deprecated transfer() can result in revert

Summary

An unsafe transfer call is used in the function checkedTransfer() in the contract Erc20CheckedTransfer.sol (utils).

Vulnerability Detail

Transfer has hard coded gas budget and can fail when the user is a smart contract. Whenever the user either fails to implement the payable fallback function or cumulative gas cost of the function sequence invoked on a native token transfer exceeds 2300 gas consumption limit the native tokens sent end up undelivered and the corresponding user funds return functionality will fail each time.

This is the reason if any transfer exceeds 2300 gas consumption limit, the function will always revert.

Impact

transfer() uses a fixed amount of gas, which can result in revert.

The function checkedTransfer() is used in ClaimFacet.sendInterests() to 'send principal plus interests of the loan to msg.sender' If this function fails, the function sendInterests() would also fail and this will result to lost of funds (interests) to users.

Code Snippet

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

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

Tool used

Manual Review

Recommendation

Use call instead of transfer().

Reference

https://consensys.net/diligence/blog/2019/09/stop-using-soliditys-transfer-now/

sherlock-audit / 2023-02-kairos-judging

w42d3n - Usage of deprecated transfer() can result in revert #168