sherlock-audit / 2024-05-pooltogether-judging

8 stars 4 forks source link

0x73696d616f - `PrizeVault::deposit/mint()` will always revert due to lossy deposits in yield vaults due to rounding errors when the yield buffer is depleted #90

Closed sherlock-admin3 closed 2 months ago

sherlock-admin3 commented 3 months ago

0x73696d616f

medium

PrizeVault::deposit/mint() will always revert due to lossy deposits in yield vaults due to rounding errors when the yield buffer is depleted

Summary

PrizeVault::deposit/mint() check if after the deposit total assets are bigger than total debt, but this will never be true for yield vaults that incurr in rounding errors and the yield buffer is depleted. Thus, depositing/minting will always be DoSed.

Vulnerability Detail

PrizeVault::deposit() and PrizeVault::mint() mint shares at a rate of 1:1 with the assets deposited. This means that if there are rounding errors, an amount of shares will be minted, but the actual precise underlying assets will be worth less than the same deposited amount of assets. Thus, the lossy deposit check will fail and the deposit will revert. Consider the following example:

assets = 0
debt = 0
deposit 1000 assets
mint 1000 shares, so debt = 1000
rounding error occurs when depositing, so
assets = 999
debt = 1000
// revert due to lossy deposit

Impact

Permanently DoSed deposits.

Code Snippet

https://github.com/sherlock-audit/2024-05-pooltogether/blob/main/pt-v5-vault/src/PrizeVault.sol#L991-L993

Tool used

Manual Review

Vscode

Recommendation

The minted shares should be minted based on the actually precise amount of assets added, instead of the assets deposited. This way the check will never revert.

Duplicate of #134

sherlock-admin4 commented 2 months ago

1 comment(s) were left on this issue during the judging contest.

infect3d commented:

expected__ see L112-114 of PrizeVault.sol