Users can be eligable for prizes with little to no risk

[c4-bot-6]

Lines of code

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

Vulnerability details

Impact

After users deposit into the PrizeVault contract their deposit is then transferred to yieldVault, however if yieldVault has reached the maximum deposit limit, the transferred tokens will stay inside the PrizeVault contract risk-free and withdrawable at all time. The problem is that they will have shares of PrizeVault and according to the documentation - "In order for users to be eligible to win prizes, their balances must be tracked in the Twab Controller". This means that users that didn't deposit into the yield vault will still be eligible for prizes because their balance in the Twab Controller is positive.

Proof of Concept

Here is how the user can perform this: (This depends on the implementation of the yieldVault and if it reverts on a zero deposit)

1. Let's look at the example in which the yield vault does allow a zero deposit

   • A user calls yieldVault.maxDeposit and sees that the vault reached the deposit limit
   • The user calls PizeVault::deposit with let's say 100e18 tokens
   • PrizeVault tries to deposit the tokens into the yield vault but since the deposit limit is reached it does nothing and returns 0 as the deposit is not accepted
   • After this the user gets minted 100e18 shares of the PrizeVault, making him eligible for prizes

2. Let's look at the example in which the yield vault does NOT allow a zero deposit

   • A user calls yieldVault.maxDeposit and sees that the vault's max deposit is 10e18 (10e18 tokens are left until the limit is reached)
   • The user calls PizeVault::deposit with 100e18 tokens
   • PrizeVault deposits only 10e18 tokens and the rest 90e18 tokens are stored in the PrizeVault. This way a user is only risking 10e18 tokens but eligible for larger rewards since he gets minted 100e18 shares

Tools Used

Manual Review

Recommended Mitigation Steps

Make it such that users cannot deposit more than PrizeVault::maxDeposit. Add the check in the deposit and mint function

Assessed type

ERC4626

[c4-pre-sort]

raymondfam marked the issue as insufficient quality report

[c4-pre-sort]

raymondfam marked the issue as primary issue

[raymondfam]

Incorrect assumption. _depositAndMint is atomic. If the maxDeposit is hit, the function will just revert and no deposit of assets is possible.

[c4-judge]

hansfriese marked the issue as unsatisfactory: Invalid