Zaros Part 1

Summary

Order fee is separated between taker and maker fee based on weather they are filling an already existing order from the orderbook or creating a new one that is waiting to be filled. Essentially they are making or taking liquidity from the pool. The way PerpMarket::getOrderFeeUsd is handling these fees is completely opposite to how they should be calculated.

Vulnerability Details

If we take a look at the comment which describes how the orderFee will be decided to be either maker or taker:

We can see that the orderFee is being treated as funding fee which aims to make the short open interest equal the long open interest. While currently the orderbook functionality has not been implemented yet in order to make the decision between maker and taker fee correct a more accurate way of deciding between them would be to charge maker fee when skew and sizeDelta have the same sign and taker fee when they have opposite signs.

The logic behind that is if the skew is positive that means there are more buy orders than sell orders. Because of that it is much more likely to fill a sell buy when placing a sell order than it is to fill a sell order by placing a buy order since there are more options of buy orders to choose from as the skew suggests.

Impact

Because of the way these fees are charged it is very likely that users will choose other dex platforms to use as their order fees will be calculated properly. The other possible impact would be that the fee will not properly try to balance the liquidity as most fees have that purpose and make the contract volatile.

Tools Used

Manual review
VS code

Recommendations

Swap completely the maker and taker fees. You can keep the decision for when skew == 0 as it is a design decision.