Vault doesn't round shares up on withdraw

Summary

In Vault.sol the code allows users to convert assets to shares and vice versa via the update function. Issue is that the conversion functions could allow malicious users to withdraw assets from the vault for free.

Vulnerability Detail

The issue arises from the use of the mulDiv function in the _socialize function of the Vault.sol contract. This function rounds down the result of the computation, meaning that if the result is less than 1, it will be rounded down to zero. This can lead to a situation where a user can withdraw assets from the vault without burning any shares.

Assume that the vault with the following state:

Total Asset = 1000 USDC
Total Supply = 10 shares

Assume that Alice wants to withdraw 99 USDC from the vault. Thus, she calls the Vault.update(… claimAssets: 99USDC)

The Vault._socialize function will compute the number of shares that Alice needs to burn in exchange for 99 USDC.

assets.mulDiv(Down)(supply, totalAssets())
99USDC.mulDiv(Down)(10 shares, 1000USDC)
(99 * 10) / 1000
990 / 1000 = 0.99 = 0

Since Solidity rounds 0.99 down to 0, Alice does not need to burn a single share. She will receive 99 USDC for free.

Impact

User can withdraw assets from the vault without burning any shares

Code Snippet

function _socialize(
    Context memory context,
    UFixed6 depositAssets,
    UFixed6 redeemShares,
    UFixed6 claimAssets
) private view returns (UFixed6 claimAmount) {
...
    claimAmount = claimAssets.muldiv(totalCollateral.min(context.global.assets), context.global.assets);
...
}

https://github.com/sherlock-audit/2023-07-perennial/blob/main/perennial-v2/packages/perennial-vault/contracts/Vault.sol/#L116

Tool used

Manual Review

Recommendation

Use muldivUp in _socialize and convertToShares function and in other mint/withdraw functions.

Rounding up in mint/withdraw is also recommended for the ERC4626 veYFI example

sherlock-audit / 2023-07-perennial-judging

tives - Vault doesn't round shares up on withdraw #125