DeFiFoundry
60,000 USDC
View results
Submission Details
Severity: low
Invalid

UnBounded orderFees

Summary

Unlimited order fees may result in unfair fees being charged to users.

Vulnerability Details

OrderFee is the Perps market maker and taker fee and is set in the createPerpMarket()/updatePerpMarketConfiguration() function in GlobalConfigurationBranch.sol.

https://github.com/Cyfrin/2024-07-zaros/blob/main/src/perpetuals/branches/GlobalConfigurationBranch.sol#L379-L441

https://github.com/Cyfrin/2024-07-zaros/blob/main/src/perpetuals/branches/GlobalConfigurationBranch.sol#L475-L541

However, as you can see in the provided code snippet, validation for orderFees is not performed.

orderFees is used to calculate order fee in SettlementBranch.sol#_fillOrder().

function getOrderFeeUsd(
Data storage self,
SD59x18 sizeDelta,
UD60x18 markPriceX18
)
internal
view
returns (UD60x18 orderFeeUsd)
{
SNIP...
if (sameSide) {
...
-> UD60x18 feeBps = isSkewGtZero != isBuyOrder && !skew.isZero()
? ud60x18(self.configuration.orderFees.makerFee)
: ud60x18(self.configuration.orderFees.takerFee);
// output order fee
-> orderFeeUsd = markPriceX18.mul(sizeDelta.abs().intoUD60x18()).mul(feeBps);
}
...
else {
// convert new skew abs(SD59x18) -> uint256
uint256 takerSize = newSkew.abs().intoUint256();
// abs( abs(orderSize) - abs(newSkew) ) -> uint256
uint256 makerSize = sizeDelta.abs().sub(sd59x18(int256(takerSize))).abs().intoUint256();
// calculate corresponding fees for maker and taker portions
// of this trade which flipped the skew
-> UD60x18 takerFee =
markPriceX18.mul(ud60x18(takerSize)).mul(ud60x18(self.configuration.orderFees.takerFee));
-> UD60x18 makerFee =
markPriceX18.mul(ud60x18(makerSize)).mul(ud60x18(self.configuration.orderFees.makerFee));
// output order fee
orderFeeUsd = takerFee.add(makerFee);
}
}

As you can see here, an unbounded order fee may result in unfair order fees being added to users.

Impact

unfair order fees being added to users.

Tools Used

Manual Review

Recommendations

It is a good idea to add min and max bounds to orderFee.

Updates

Lead Judging Commences

inallhonesty Lead Judge
about 1 year ago
inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.