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

0 stars 0 forks source link

`redeemToken` can fail for certain tokens #61

Open code423n4 opened 3 years ago

code423n4 commented 3 years ago

Handle

cmichel

Vulnerability details

The SwappableYieldSource.redeemToken function transfers tokens from the contract back to the sender, however, it uses the ERC20.transferFrom(address(this), msg.sender, redeemableBalance) function for this. Some deposit token implementations might fail as transferFrom checks if the contract approved itself for the redeemableBalance instead of skipping the allowance check in case the sender is the from address.

This can make the transaction revert and the deposited funds will be unrecoverable for the user.

It's recommended to use _depositToken.safeTransfer(msg.sender, redeemableBalance) instead.

PierrickGT commented 3 years ago

Duplicate of https://github.com/code-423n4/2021-07-pooltogether-findings/issues/25

0xean commented 3 years ago

re-opening this issue and marking #25 as a duplicate of this issue which clearly articulates the potential severity of unrecoverable user funds.

PierrickGT commented 3 years ago

This issue has been fixed and we are now using safeTransfer: https://github.com/pooltogether/swappable-yield-source/blob/bf943b3818b81d5f5cb9d8ecc6f13ffecd33a1ff/contracts/SwappableYieldSource.sol#L235