Fujicracy / fuji-v2

Cross-chain money market aggregator
https://fuji-v2-frontend.vercel.app
15 stars 10 forks source link

Unsafe usage of ERC20 transfer #318

Closed cvetanovv closed 1 year ago

cvetanovv commented 1 year ago

Summary

The ERC20.transfer() function return a boolean value indicating success. This parameter needs to be checked for success. Some tokens do not revert if the transfer failed but return false instead.

Vulnerability Detail

Using unsafe ERC20 methods can revert the transaction for certain tokens.

Impact

Tokens that don't actually perform the transfer and return false are still counted as a correct transfer and tokens that don't correctly implement the latest EIP20 spec will be unusable in the protocol as they revert the transaction because of the missing return value.

Code Snippet

https://github.com/Fujicracy/fuji-v2/blob/v0.0.1/packages/protocol/src/abstracts/BaseRouter.sol#L106 https://github.com/Fujicracy/fuji-v2/blob/v0.0.1/packages/protocol/src/flashloans/FlasherEuler.sol#L60

Recommendation

Recommend using OpenZeppelin's SafeERC20 versions with the safeTransfer and safeTransferFrom functions that handle the return value check as well as non-standard-compliant tokens.

0xdcota commented 1 year ago

BaseRouter was fixed in commit FlashEuler is addressed in pull request #553.