ERC20 return values not checked

[code423n4]

Handle

cmichel

Vulnerability details

The ERC20.transfer() and ERC20.transferFrom() functions 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.

The Manager.recoverToken function does not check the return value of this function.

Impact

Tokens that don't actually perform the transfer and return false are still counted as a correct transfer. Furthermore, tokens that do not correctly implement the EIP20 standard, like USDT which does not return a success boolean, will revert.

Recommended Mitigation Steps

We 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.

[Haz077]

Same as #5

[GalloDaSballo]

Agree with finding, using a non reverting token can potentially cause issues to the protocol accounting

Sponsor can check each token on a case by case basis, or simply use OpenZeppelin's safeERC20