Submitted by Aymen0909, also found by cheatc0d3, turvy_fuzz, trachev, FastChecker, Tripathi, Abdessamed, 0xmystery, btk, and Giorgio
https://github.com/code-423n4/2024-03-pooltogether/blob/main/pt-v5-vault/src/PrizeVault.sol#L454-L472
https://github.com/code-423n4/2024-03-pooltogether/blob/main/pt-v5-vault/src/PrizeVault.sol#L355-L366
https://github.com/code-423n4/2024-03-pooltogether/blob/main/pt-v5-vault/src/PrizeVault.sol#L489-L508
When a user who has deposited assets into the PrizeVault wishes to withdraw (or redeem) them, they can do so by calling either the withdraw or redeem functions.
Under normal conditions in the vault, users expect to receive their full deposited asset amount back, as there is a 1:1 exchange ratio between the asset amount and shares.
However, if the underlying yield vault experiences a loss, this exchange rate will decrease. This is highlighted in the previewWithdraw function (or previewRedeem/convertToAssets function):
As shown, if the underlying yield vault experiences a loss, the total assets of the vault given by totalAssets() will decrease and might go below the total shares value totalDebt().
This will trigger the second if statement block, in which the calculated asset (or shares) amount will be converted using the ratio totalAssets/totalDebt (or its inverse for shares).
When a user redeems (or withdraws), if the yield vault experiences a loss while their transaction is pending (waiting in the mempool) and the total assets drop below the total vault debt, they will receive fewer assets after calling either the withdraw/redeem functions than they expected. Instead of a 1:1 ratio, they will use the current totalAssets/totalDebt ratio.
To illustrate this issue, consider the following scenario:
If Bob had known that the yield vault had experienced a loss, he would have waited until the exchange rate increased again to withdraw his full amount.
This issue is also present in the withdraw function, but in that case, the user will be burning more shares to get the same amount and still incurring a loss.
Both withdraw/redeem functions lack slippage protection, which can lead to users losing funds in the event of a yield vault loss.
VS Code
Both withdraw/redeem functions should include slippage protection parameters provided by the users (either minimum amount out for redeem function or maximum shares in for withdraw function).
Context
trmid (PoolTogether) confirmed and commented:
d3e4 (warden) commented:
btk (warden) commented:
hansfriese (judge) commented:
trmid (PoolTogether) commented:
