code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-06-yieldy/blob/524f3b83522125fb7d4677fa7a7e5ba5a2c0fe67/src/contracts/LiquidityReserve.sol#L188-L209

Vulnerability details

Impact

One of the tokens supported by this project is USDC, which is an upgradeable contract, and the code specifically casts addresses to IERC20Upgradeable rather than to IERC20, so the intention is for the code to support upgrades. If USDC ever upgrades to have a fee-on-transfer, rebasing behavior, or some other non-standard behavior, the contracts in this project will break, locking user funds in the state that they were before the upgrade.

Proof of Concept

For example, if an upgrade starts fee-on-transfer behavior, this function will revert, because it assumes _amount is available once claimeWithdraw() has happened, which will not be the case if a fee is charged:

File: src/contracts/LiquidityReserve.sol   #X

188       function instantUnstake(uint256 _amount, address _recipient)
189           external
190           onlyStakingContract
191       {
192           require(isReserveEnabled, "Not enabled yet");
193           // claim the stakingToken from previous unstakes
194           IStaking(stakingContract).claimWithdraw(address(this));
195   
196           uint256 amountMinusFee = _amount - ((_amount * fee) / BASIS_POINTS);
197   
198           IERC20Upgradeable(rewardToken).safeTransferFrom(
199               msg.sender,
200               address(this),
201               _amount
202           );
203   
204           IERC20Upgradeable(stakingToken).safeTransfer(
205               _recipient,
206               amountMinusFee
207           );
208           unstakeAllRewardTokens();
209       }

https://github.com/code-423n4/2022-06-yieldy/blob/524f3b83522125fb7d4677fa7a7e5ba5a2c0fe67/src/contracts/LiquidityReserve.sol#L188-L209

There are a lot of other examples where the balances aren't checked

Tools Used

Code inspection

Recommended Mitigation Steps

Measure balances before and after transfers, and use the difference as the amount rather than the stated amount.

toshiSat commented 2 years ago

We will update the documentation to include these contracts will not work for ERC20 tokens that have fees

KenzoAgada commented 2 years ago

In the judging sheet this was judged as unique, but looks like a duplicate of M-17 #222

code-423n4 / 2022-06-yieldy-findings