Missing return values in IERC20.transferFrom.

[c4-bot-10]

Lines of code

https://github.com/kkrt-labs/kakarot/blob/7411a5520e8a00be6f5243a50c160e66ad285563/src/backend/starknet.cairo#L264

Vulnerability details

Summary

Missing Return Values in IERC20.transferFrom.

ERC20 token behaviors in scope

Missing return values: In scope

Vulnerability details

Some tokens do not return a bool (e.g. USDT, BNB, OMG) on ERC20 methods. see here for a comprehensive (if somewhat outdated) list.
Some tokens (e.g. BNB) may return a bool for some methods, but fail to do so for others. This resulted in stuck BNB tokens in Uniswap v1 (details). Some particularly pathological tokens (e.g. Tether Gold) declare a bool return, but then return false even when the transfer was successful (code).
A good safe transfer abstraction (example) can help somewhat, but note that the existence of Tether Gold makes it impossible to correctly handle return values for all tokens.

Proof Of Concept

As seen below, there is a missing return value as transferFrom. It does not return a bool for the ERC20 token transfer operation.

        IERC20.transferFrom(
            token_address, transfer.sender.starknet, transfer.recipient.starknet, transfer.amount
        );

Tools Used

Manual Review

Recommendation Mitigaiton Steps

The protocol should use the safe transfer method from open zeppelin.

        IERC20.safeTransferFrom(
            token_address, transfer.sender.starknet, transfer.recipient.starknet, transfer.amount
        );

Assessed type

ERC20