Part 2

Zaros
PerpetualsDEXFoundrySolidity
70,000 USDC
View results
Submission Details
Severity: high
Valid

When there is a positive PnL event, deleveraging will be applied twice

Summary

In the SettlementBranch.sol the auto-deleveraging will be applied twice.

Vulnerability Details

In the _filOrder function auto-deleveraging is applied when the old position of a trader had a positive pnl:

// 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);
1) ctx.marginToAddX18 =
marketMakingEngine.getAdjustedProfitForMarketId(marketId, ctx.pnlUsdX18.intoUD60x18().intoUint256());
2) 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
3) marketMakingEngine.withdrawUsdTokenFromMarket(marketId, ctx.marginToAddX18.intoUint256());
}

However in point 1) in the code above we can see that auto deleveraging will first be applied in the getAdjustedProfitForMarketId:

function getAdjustedProfitForMarketId(
uint128 marketId,
uint256 profitUsd
)
public
view
returns (UD60x18 adjustedProfitUsdX18)
{
...
// 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);
}
}

This will return the amount with the applied deleveraging. This amount will be deposited to the trading account in point 2).
In point 3) we see that the withdrawUsdTokenFromMarket is called. However the amount that is passed in the function is the deleveraged amount.
When we check out the withdrawUsdTokenFromMarket we can see that the this amount will be deleveraged once again:

function withdrawUsdTokenFromMarket(uint128 marketId, uint256 amount) external onlyRegisteredEngine(marketId) {
...
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());
} else {
// if the market is not in the ADL state, it realizes the full requested USD Token amount
market.updateNetUsdTokenIssuance(amountX18.intoSD59x18());
}
// loads the market making engine configuration storage pointer
MarketMakingEngineConfiguration.Data storage marketMakingEngineConfiguration =
MarketMakingEngineConfiguration.load();
// mint USD Token to the perps engine
UsdToken usdToken = UsdToken(marketMakingEngineConfiguration.usdTokenOfEngine[msg.sender]);
>>> usdToken.mint(msg.sender, amountToMint);
// emit an event
emit LogWithdrawUsdTokenFromMarket(msg.sender, marketId, amount, amountToMint);

Also the amount that was deleveraged twice will be minted to the engine, while the user's position was updated with the amount that was deleveraged only once.

Root cause

The withdrawUsdTokenFromMarketuses the deleveraged amount instead of the original `pnlUsdX18`

Impact

As a result the protocol will even be in an insolvent state as there will be more amount recorded(the amount that was deleveraged only once) in the positions of the perp engine than it was minted to it(the amount that was deleveraged twice).

Tools Used

Recommendations

Currently the amount will be deleveraged twice in getAdjustedProfitForMarketId and then in withdrawUsdTokenFromMarket. To fix this issue you could use the original amount as an input in the withdrawUsdTokenFromMarket instead of the deleveraged one(assuming both functions will perform the same deleveraging).

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.