Liquidity Management

Summary

In PerpetualVault-> the functions mintand handleReturnare relied on for determing the amount of shares to mint to users, and the amount to send to users based on their shares at the time of withdraw, respectively.

The two functions use a different and inconsistent method for determining value: Which can be a design choice, but if it is, it is not executed properly

• _mintuses total collateral balance at the time of initiating deposit, not including the users deposit in the total collateral balance value

• _handleReturntries to use the total collateral balance that includes the users withdrawn amount, and not the total collateral balance at the time of initiating the withdraw. There is doubt that it is the design choice to do so, but even if it is the design choice, the calculation is wrong and will lead to user getting a very inflated value transferred to them when the position is open.

For this finding, I will assume it is protocol design decision to use a different method for withdraw, which adds the withdrawn amount to total collateral balance, which is used in determing the value of the shares. And not using the total collateral balance at the time of user initiating the withdraw.

Vulnerability Details

First, lets look at how the _mintfunction determines the amount of shares to mint to the user based on their deposit amount - by using totalAmountBeforewhich effectively uses the balance of the contract at the time of initiating the deposit (not including the newly deposited amount by the user) -> it subtracts the newly deposited amount from the total balance of the contract to achieve this.

** CHECK MY IN LINE COMMENTS for more effective communication

The issue is in:

_handleReturn-> which is the end of the user's withdrawal flow, and is used to transfer the determined value amount of the users shares back to the user, and burns their shares. There are 2 different scenarios handled :

• when the protocol has a closed position

• when the protocol has an open position

** When the position is closed:

• calculates amount, using the total collateral balance of the contract (which includes the users withdrawn amount, becasue it was sent in from GMX prior to this when the decrease position was executed)

** When the position is open:

• the total collateral balance of the contract at the time of the user initiating withdraw is calculated via balanceBeforeWithdraw

  The way and problem with how amountis calculated:

  • 1st: -> becasue of the order of operations of the equation balanceBeforeWithdrawal * shares / totalShareswill be calculated first

  • 2nd: -> withdrawnis added to that calculated value

As a result, the calculated values for amountwhen the position is closed or open IS VERY DIFFERENT

Following the previous methods for determing value used by the protocol -> by using: colalteral balance, shares, and total shares -> the calculated value for amountWILL BE VERY DIFFERENT than what the rest of the protocol produces and expects

** Check inline comments for clearer communication

Impact

For the sake of simplicity, just to illustrate the wrong calculation when the position is open:

• total collateral = 100,000

• withdrawn = 20,000

• shares = 10

• totalShares = 50

When the position is closed

• amount = 100,000 * 10 / 50

• = 20,000

When the position is open

• balanceBeforeWithdraw= 80,000

• 20,000 + 80,000 * 10 / 50

• First the equation will calulate -> 80,000 * 10 / 50

• = 16,000

• NEXT it will add 20,000 to that value

• amount= 20,000 + 16,000

• amount= 36,000

When the position is open, the user will get their withdrawn amount added to the value of their shares - essentially almost giving the user DOUBLE the amount they should receive.

Tools Used

Manual Review

Recommendations

For the correct calculation in _handleReturn-> the withdrawnamount needs to be added to balanceBeforeWithdrawbefore the rest of the equation continues, to ensure the correct order of operations.