PoolTogether - Giorgio's results

Lines of code

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

Vulnerability details

When debt accrues, the PrizeVault share will be worth less assets, consequently this could lead users withdrawing less assets than expected

Impact

Users will withdraw less assets or redeem more shares than expected.

Proof of Concept

When a user want to withdraw assets, the amount of shares needed are calculated through the previewWithdraw() function.

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

The previewWithdraw will return a less favorable rate for the users if (_totalAssets >= totalDebt_), normally to shares to asset ratio is 1:1, but in this case more shares will be needed: return _assets.mulDiv(totalDebt_, _totalAssets, Math.Rounding.Up); .

Which means that if the debt momentarily increases the value of a PrizeVault share will decrease and thus the user will withdraw less assets, which results in a loss for him.

Tools Used

Manual review

Recommended Mitigation Steps

In order to mitigate any slippage issues, add a check for both the withdraw() and redeem() functions.

   -- function withdraw( uint256 _assets, address _receiver, address _owner) 
   ++ function withdraw( uint256 _assets, address _receiver, address _owner, uint256 maxShares) 
external returns (uint256) {
        uint256 _shares = previewWithdraw(_assets);
 ++     require(_shares <= maxShares, "slippage error"); 
        _burnAndWithdraw(msg.sender, _receiver, _owner, _shares, _assets);
        return _shares;
    }

    function redeem( 
     -- uint256 _shares, address _receiver, address _owner
     ++ uint256 _shares, address _receiver, address _owner, uint256 minAssets
    ) external returns (uint256) {
        uint256 _assets = previewRedeem(_shares);
    ++  require(_assets >= minAssets, "slippage err");
        _burnAndWithdraw(msg.sender, _receiver, _owner, _shares, _assets);
        return _assets;
    }

Assessed type

Other