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

5 stars 4 forks source link

`PrizeVault` is not fully compliant with the ERC4626 standard #339

Closed c4-bot-2 closed 5 months ago

c4-bot-2 commented 5 months ago

Lines of code

https://github.com/code-423n4/2024-03-pooltogether/blob/480d58b9e8611c13587f28811864aea138a0021a/pt-v5-vault/src/PrizeVault.sol#L341-L352

Vulnerability details

Vulnerability Details

The 4626 EIP specifies that in order for a contract to be fully compliant with the standard, the convertToShares function must not revert, except due to an integer overflow. The issue is that it is currently possible for PrizeVault's convertToShares function to revert due to a division by 0. As if the total assets are less than the total debt, the function returns _assets.mulDiv(totalDebt_, _totalAssets, Math.Rounding.Down);. Therefore, if _totalAssets is 0 and totalDebt_ is more than 0 the function will revert. This is possible in the case that all users have withdrawn their deposits, but the fee recipient is yet to claim their yield fees and yieldFeeBalance is more than 0. The fact that yieldFeeBalance is more than 0 does not mean that the total assets will be more than 0, due to the fact that _totalAssets can always be decreased because of fees, slippage, or loss of funds in the underlying yield vault.

Impact

PrizeVault is not fully compliant with the ERC4626 standard.

Tools Used

Manual review

Recommended Mitigation Steps

In converToShares if _totalAssets is 0 and totalDebt_ is a positive number return 0.

Assessed type

ERC4626

c4-pre-sort commented 5 months ago

raymondfam marked the issue as insufficient quality report

c4-pre-sort commented 5 months ago

raymondfam marked the issue as duplicate of #71

c4-judge commented 5 months ago

hansfriese marked the issue as unsatisfactory: Insufficient proof