code-423n4 / 2024-03-pooltogether-findings

5 stars 4 forks source link

No slippage control in the `PrizeVault.sol#withdraw()` and `PrizeVault.sol#redeem()` functions. The lack of slippage control for `PrizeVault.sol#withdraw()`, `PrizeVault.sol#redeem()` function can lead to a loss of assets for the affected users. #270

Closed c4-bot-9 closed 7 months ago

c4-bot-9 commented 7 months ago

Lines of code

https://github.com/code-423n4/2024-03-pooltogether/blob/main/pt-v5-vault/src/PrizeVault.sol#L489-L497 https://github.com/code-423n4/2024-03-pooltogether/blob/main/pt-v5-vault/src/PrizeVault.sol#L500-L508

Vulnerability details

Impact

Loss of assets for the affected users.

Proof of Concept

The PrizeVault.sol#withdraw() and PrizeVault.sol#redeem() functions call the PrizeVault.sol#previewWithdraw() and PrizeVault.sol#previewRedeem() functions to calculate the amount of stocks to be burned or assets to be redeemed. When users deposit into PrizeVault they are really depositing into an ERC4626 vault that contributes its yield to the Prize Pool. Therefore, when executing a transaction, the entire assets of PrizeVault can rise or fall at any time depending on the current onchain status according to the stock value of the yield vault.

    function totalAssets() public view returns (uint256) {
        return yieldVault.convertToAssets(yieldVault.balanceOf(address(this))) + _asset.balanceOf(address(this));
    }

Therefore, if PrizeVault's total assets become less than totalDebt, slippage may occur in these functions(previewWithdraw(), previewRedeem()). This slippage is more than what they can accept. In summary, the PrizeVault.sol#withdraw(), PrizeVault.sol#redeem() functions lack slippage control that allows the user to revert if the amount of shares the user redeems is greater than expected or if the amount of assets the user receives is less than expected.

Tools Used

Manual Review

Recommended Mitigation Steps

Implement slip control in PrizeVault.sol#withdraw(), PrizeVault.sol#redeem() functions.

Assessed type

Other

c4-pre-sort commented 7 months ago

raymondfam marked the issue as sufficient quality report

c4-pre-sort commented 7 months ago

raymondfam marked the issue as duplicate of #90

c4-pre-sort commented 7 months ago

raymondfam marked the issue as duplicate of #274

c4-judge commented 7 months ago

hansfriese marked the issue as satisfactory