search

code-423n4 / 2021-06-pooltogether-findings

0 stars 0 forks source link

`SushiYieldSource` ERC20 return values not checked #88

Closed code423n4 closed 3 years ago

code423n4 commented 3 years ago

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.

It is not checked in SushiYieldSource.supplyTokenTo.

Impact

Tokens that don't actually perform the transfer and return false are still counted as a correct transfer. As the sushiAddr is merely a parameter to the yield source it is not known which token & SushiBar will end up actually being used.

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.

asselstine commented 3 years ago

See https://github.com/code-423n4/2021-06-pooltogether-findings/issues/112

Won't break because the subsequent deposit() calls will fail. No advantage to be gained; just bad logic.

dmvt commented 3 years ago

duplicate of #112