code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-04-dualityfocus/blob/f21ef7708c9335ee1996142e2581cb8714a525c9/contracts/vault_and_oracles/FlashLoan.sol#L57

Vulnerability details

Impact

The FlashLoan.recoverToken function does not check the return value of the transferFrom function executed on a provided token.

The IERC20.transferFrom() 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.

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.

Proof of Concept

https://github.com/code-423n4/2022-04-dualityfocus/blob/f21ef7708c9335ee1996142e2581cb8714a525c9/contracts/vault_and_oracles/FlashLoan.sol#L57

Tools Used

Slither

Recommended Mitigation Steps

It is recommended to use OpenZeppelin’s SafeERC20 with the safeTransferFrom function that handles the return value check as well as non-standard-compliant tokens.

0xdramaone commented 2 years ago

Duplicate of #40

JeeberC4 commented 2 years ago

Generating QA Report as warden did not submit one and judge downgraded issue. Preserving original title: ERC20 Return Values Not Check

code-423n4 / 2022-04-dualityfocus-findings