Summary

Now the maintaince margin check in _fillOrder is too early and not sufficient, certain positions may be able to be liquidated instantly after settlement.

Vulnerability Details

For users who wish to change their existing positions, the following is the logic executed by _fillOrder: (We omit some security checks for now)

1. fetch global config, perp market's config, account config, etc from storage

2. get funding rates for this perp market (using fillprice)

3. update funding rates

4. calculate order & settlement fees (using fillprice)

5. calculate required initial & maintenance margin for this trade and account's unrealized PNL (using IndexPrice)

6. Ensure that the user has sufficient margin (based on calculations in step 5)

7. cache unrealized PNL from potential existing position in this market including accrued funding (using fillprice)

8. update all config storage

9. If unrealized PnL > 0, mint new usdTokens for trader's margin account

10. Deduct collateral ( If unrealized PnL < 0 ) and order/settlement fees from trader's margin account

Now lets dive into step 5 to figure out what happeds here:

2024-07-zaros/src/perpetuals/leaves/TradingAccount.sol at main · Cyfrin/2024-07-zaros · GitHub

Whether for targetMarketId or another Market, the markPrice used is always based on Chainlink Feed.

There are two central points of conflict here:

1. The cost of this order(order/settlement fees) was not taken into account when calculating the margin in step 5, but they are later subtracted from the margin in step 10.

2. The price used in Step 5(to calculate markPrice) is indexPrice, while the price used in step 7(to calculate markPrice) is fillprice, which is one of ASK/BID price. There is often a spread between these three prices, which makes the actual PnL and the estimated value differ significantly

Impact

Likelihood: medium - This issue require certain position size and margin balance to exist.

+

Impact: high - 1. Some positions can be liquidated immediately after settlement 2. Protocol can be insolvent if the liquidation is too deep such that the accounts collateral is not enough to cover the potential losses.

= Severity: high

Tools Used

Recommendations

check if the opened position will make the account liquidatable at the end of the execute order.