Part 2
Zaros
PerpetualsDEXFoundrySolidity
70,000 USDC
View results
Previous Next
Submission Details
Severity: high
Valid

Trader receives less credit due to deleveraging factor being applied twice

greese

Summary

When an order is filled through _fillOrder, if the trader has a positive pnl they receive credit and receive settlement tokens from the market making engine. The problem is the margin to add is deducted by the delevergaing factor twice leading to less settlement tokens minted to the trader than intended. Once through getAdjustedProfitForMarketId and again through withdrawUsdTokenFromMarket

Vulnerability Details

A market order keeper intends to fill an order. When _fillOrder is called it checks if the trader's pnl is currently positive. If it is they are credited settlement tokens through the market making engine. The margin to add needs to be adjusted based on the deleveraging factor. That is done through the call to getAdjustedProfitForMarketId. The amount returned is then passed into withdrawUsdTokenFromMarket to mint the corresponding amount of settlement tokens.

// if trader's old position had positive pnl then credit that to the trader
if (ctx.pnlUsdX18.gt(SD59x18_ZERO)) {
IMarketMakingEngine marketMakingEngine = IMarketMakingEngine(perpsEngineConfiguration.marketMakingEngine);
​
ctx.marginToAddX18 =
marketMakingEngine.getAdjustedProfitForMarketId(marketId, ctx.pnlUsdX18.intoUD60x18().intoUint256());
​
tradingAccount.deposit(perpsEngineConfiguration.usdToken, ctx.marginToAddX18);
​
// mint settlement tokens credited to trader; tokens are minted to
// address(this) since they have been credited to the trader's margin
marketMakingEngine.withdrawUsdTokenFromMarket(marketId, ctx.marginToAddX18.intoUint256());
}

The problem is that both of these functions are reducing the amount by the deleveraging factor.
https://github.com/Cyfrin/2025-01-zaros-part-2/blob/35deb3e92b2a32cd304bf61d27e6071ef36e446d/src/market-making/branches/CreditDelegationBranch.sol#L154C9-L160C10
https://github.com/Cyfrin/2025-01-zaros-part-2/blob/35deb3e92b2a32cd304bf61d27e6071ef36e446d/src/market-making/branches/CreditDelegationBranch.sol#L283C9-L292C64

// we don't need to add `profitUsd` as it's assumed to be part of the total debt
// NOTE: If we don't return the adjusted profit in this if branch, we assume marketTotalDebtUsdX18 is positive
if (market.isAutoDeleverageTriggered(delegatedCreditUsdX18, marketTotalDebtUsdX18)) {
// if the market's auto deleverage system is triggered, it assumes marketTotalDebtUsdX18 > 0
adjustedProfitUsdX18 =
market.getAutoDeleverageFactor(delegatedCreditUsdX18, marketTotalDebtUsdX18).mul(adjustedProfitUsdX18);
}
// now we realize the added usd debt of the market
// note: USD Token is assumed to be 1:1 with the system's usd accounting
if (market.isAutoDeleverageTriggered(delegatedCreditUsdX18, marketTotalDebtUsdX18)) {
// if the market is in the ADL state, it reduces the requested USD
// Token amount by multiplying it by the ADL factor, which must be < 1
UD60x18 adjustedUsdTokenToMintX18 =
market.getAutoDeleverageFactor(delegatedCreditUsdX18, marketTotalDebtUsdX18).mul(amountX18);
​
amountToMint = adjustedUsdTokenToMintX18.intoUint256();
market.updateNetUsdTokenIssuance(adjustedUsdTokenToMintX18.intoSD59x18());

This greatly reduces the intended amount of credit dedicated to this user.

Impact

Loss of user credit

Tools Used

Manual Review

Recommendations

There is no need to check the deleveraging factor when we mint the tokens in withdrawUsdTokenFromMarket as it is already accounted for.

Updates

Lead Judging Commences

inallhonesty Lead Judge 6 months ago
Submission Judgement Published
Validated
Assigned finding tags:

`SettlementBranch._fillOrder` profit adjustment is applied to positive PnL twice.

Support

FAQs

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