Update margin uses new leverage for balance release
Description
The function updateMargin
allows users to deposit or withdraw USDC from their open positions. When updating the margin order, there are fees (marginFees
) that the users have to pay. These fees are allocated as rewards in the vault manager, and as the fees are taken from the origin order, it is essential to release that balance and update the open interest related to the fees.
The function updateMargin
stores the pending margin-update order using the function storePendingMarginUpdateOrder
and stores the new leverage in the struct's oldLeverage
field. Later, in the callback function updateMarginCallback
, the o.marginFees.mul(o.oldLeverage)
value is used to release the balance and update the open interest.
As o.oldLeverage
is the new leverage and not the old leverage, it would release an incorrect amount of balance, and the open interest would also be updated using this incorrect value.
Impact
If more tokens are released from the vault, in the long term it could lead to losing positions not being liquidated as there will not be enough balance in the vault to be released.
Also, using the new leverage and trade size makes the calculation for the withdrawal threshold check incorrect when it interacts with the -100% loss cap.
For example, say the original investment was $100 with leverage 100x, the loss-protection multiplier is 80%, and the asset price changes by -0.5%. Now, the position has a PNL of -$50, which is -$40 after the loss-protection multiplier, so the position is worth $60.
But, consider if the user tries to withdraw $75 minus a WEI, so the new position is $25 plus a WEI and the new leverage is slightly less than 400x. Now, because the leverage is higher, the percent profit will be -200%, reduced to -100% after the loss cap and then reduced to -80% due to the loss protection tier, so it is $20. The require statement checks if $25 + a WEI - $20 > ($25 + a WEI) * 80%, which is true, so it allows the user to withdraw the amount despite it being more than what the position is worth.
Recommendations
Pass the correct old leverage value in the storePendingMarginUpdateOrder
function. Ensure that the callback correctly uses the old value when checking if the withdraw threshold is breached.
Remediation
This issue has been acknowledged by Avantis Labs, Inc., and a fix was implemented in commit d625039e↗.