Slippage controls for calling `PrizeVault::deposit` and `PrizeVault::mint` functions are missing

[c4-bot-5]

Lines of code

https://github.com/code-423n4/2024-03-pooltogether/blob/main/pt-v5-vault/src/PrizeVault.sol#L475-L479 https://github.com/code-423n4/2024-03-pooltogether/blob/main/pt-v5-vault/src/PrizeVault.sol#L482-L486

Vulnerability details

https://eips.ethereum.org/EIPS/eip-4626 mentions that "if implementors intend to support EOA account access directly, they should consider adding an additional function call for deposit/mint/withdraw/redeem with the means to accommodate slippage loss or unexpected deposit/withdrawal limits, since they have no other means to revert the transaction if the exact output amount is not achieved."

Using the PrizeVault contract that inherits the ERC4626DepositOnly contract, EOAs can call the ERC4626DepositOnly.deposit and ERC4626DepositOnly.mint functions directly. However, because no slippage controls can be specified when calling these functions, these function's shares and assets outputs can be less than expected to these EOAs.

Impact

These function's shares and assets outputs can be less than expected to these EOAs.

Proof of Concept

The following steps can occur for the described scenario involving the PrizeVault contract's ERC4626DepositOnly.mint function. The case involving the PrizeVault contract's ERC4626DepositOnly.deposit function is similar to this:

Alice wants to mint 1e18 shares in exchange for sending and burning 1e18 tokens. Alice calls the contract's ERC4626DepositOnly.mint function with the shares input being 1e18. Yet, such ERC4626DepositOnly.mint function call causes 1.2e18 tokens to be transferred from Alice. Alice unexpectedly sends, burns, and loses 0.2e18 more tokens than expected for minting 1e18 shares.

Tools Used

Manual review

Recommended Mitigation Steps

The PrizeVault contract can be updated to include a deposit function that allows msg.sender to specify the minimum shares to be minted for calling the corresponding ERC4626DepositOnly.deposit function; calling such PrizeVault.deposit function should revert if the corresponding ERC4626DepositOnly.deposit function's shares output is less than the specified minimum shares to be minted. Similarly, the PrizeVault contract can also include a mint function that allows msg.sender to specify the maximum tokens to be sent for calling the corresponding ERC4626DepositOnly.mint function; calling such PrizeVault.mint function should revert if the corresponding ERC4626DepositOnly.mint function's assets output is more than the specified maximum tokens to be sent.

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]

Minting is always 1: 1 and debarred in a lossy state. So no slippage is needed.

[c4-judge]

hansfriese marked the issue as unsatisfactory: Invalid