Liquidity Management

Summary

The _mint function is called at the end of a deposit flow to mint shares to users in accordance to their deposits.
It is designed a way such that when the position is closed or has not yet been opened, depositors get a fair amount of shares according to their deposited amounts, regardless of who came first.

However, a user can ensure other users are negatively impacted by making a malicious donation to the vault. He also ensures himself a greater piece of all shares minted.

Vulnerability Details

The vulnerability lies in how the memory variable totalAmountBefore is derived.

When the position is closed (or has not yet been opened), totalAmountBefore calls function _totalAmount(), which will return the vault's collateralToken balance.
This balance can be manipulated with donations that go around the deposit function.

Impact

The first deposit after the donation will not be affected, however every subsequent deposit will.
Indeed the attacker's donations would be lost, but the donated amount will be part of the total position collateral.
Additionally, it affects the total amount of shares minted.

The impact could be negatively significant for other users and the profit great for the attacker.
If the opened position returns profit, users will not benefit as much as they should, while Bob will benefit more then is fair.

This can be done at ay stage while the position is closed, however it will only affect subsequent deposits.

[Please note that the same concept applies where ever balances are used in the mint() function instead of deposit amounts and swapped amounts (applicable to other position types).]

Tools Used

Manual Review: for spotting
Foundry: for testing

Proof of Code

The following test shows how this can be done. Bob ends up owning 50% of all shares minted, rather than 33%. The other users get minted half the shares they would be entitled to for the same deposit.
Additionally, in this example, total shares emitted should have been 3^(26) rather than 2^(26).

Paste the following in PerpetualVault.t.sol:

Recommendations

Ensure that, when positionIsClosed = true, function _totalAmount returns totalDepositAmount instead of the vault's balance:

Please note that the same concept applies where ever balances are used in the mint() function instead of deposit amounts and swapped amounts (applicable to other position types).