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

5 stars 4 forks source link

No slippage protection when withdrawing and redeeming #322

Closed c4-bot-4 closed 5 months ago

c4-bot-4 commented 5 months ago

Lines of code

https://github.com/code-423n4/2024-03-pooltogether/blob/main/pt-v5-vault/src/PrizeVault.sol#L493 https://github.com/code-423n4/2024-03-pooltogether/blob/main/pt-v5-vault/src/PrizeVault.sol#L504

Vulnerability details

Impact

The withdraw and redeem operations in PrizeVault are exposed to high slippage returns and could result in a loss for LPs of PrizeVault.

Proof of Concept

When withdraw or redeem is called, they both work in similar ways in returning the amount to be burnt or received. When all assets are below the debt, the amount returned could vary. However, there is no slippage protection for these operations.

function withdraw(
        uint256 _assets,
        address _receiver,
        address _owner
    ) external returns (uint256) {
        uint256 _shares = previewWithdraw(_assets);
        _burnAndWithdraw(msg.sender, _receiver, _owner, _shares, _assets);
        return _shares;
    }

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

    function redeem(
        uint256 _shares,
        address _receiver,
        address _owner
    ) external returns (uint256) {
        uint256 _assets = previewRedeem(_shares);
        _burnAndWithdraw(msg.sender, _receiver, _owner, _shares, _assets);
        return _assets;
    }

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

However, there are no parameters for amountOutMin or amountInMax, which are used to prevent slippage. These parameters should be checked to create slippage protections.

https://docs.uniswap.org/contracts/v3/guides/providing-liquidity/decrease-liquidity

Tools Used

Visual Studio Code

Recommended Mitigation Steps

There are no parameters for amountOutMin or amountInMax, which are used to prevent slippage for varying return amount. These parameters should be checked to create slippage protections.

https://docs.uniswap.org/contracts/v3/guides/providing-liquidity/decrease-liquidity

Assessed type

MEV

c4-pre-sort commented 5 months ago

raymondfam marked the issue as sufficient quality report

c4-pre-sort commented 5 months ago

raymondfam marked the issue as duplicate of #90

c4-pre-sort commented 5 months ago

raymondfam marked the issue as duplicate of #274

c4-judge commented 5 months ago

hansfriese changed the severity to 2 (Med Risk)

c4-judge commented 5 months ago

hansfriese marked the issue as satisfactory