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

`_fillOrder` should update the vaults before deleveraging

novaman33

Summary

In the `SettlementBranch.sol` the deleverage amount is accounted incorrectly.

Vulnerability Details

In the _fillOrder function the getAdjustedProfitForMarketId is used to preview the marginToAddX18 with the applied deleveraging:

function _fillOrder(
uint128 tradingAccountId,
uint128 marketId,
uint128 settlementConfigurationId,
SD59x18 sizeDeltaX18,
UD60x18 fillPriceX18
)
internal
virtual
{
​
// 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);
​
}

However we can see that the getAdjustedProfitForMarketId does not call recalculateVaultsCreditCapacity to update the vault's capacity which has a direct effect on the total deleged credit. As a result it will use an outdated value for the creditCapacityUsdX18:

function getAdjustedProfitForMarketId(
uint128 marketId,
uint256 profitUsd
)
public
view
returns (UD60x18 adjustedProfitUsdX18)
{
// load the market's data storage pointer & cache total debt
Market.Data storage market = Market.loadLive(marketId);
SD59x18 marketTotalDebtUsdX18 = market.getTotalDebt();
​
// caches the market's delegated credit & credit capacity
UD60x18 delegatedCreditUsdX18 = market.getTotalDelegatedCreditUsd();
>>> SD59x18 creditCapacityUsdX18 = Market.getCreditCapacityUsd(delegatedCreditUsdX18, marketTotalDebtUsdX18);
​
// if the credit capacity is less than or equal to zero then
// the total debt has already taken all the delegated credit
if (creditCapacityUsdX18.lte(SD59x18_ZERO)) {
revert Errors.InsufficientCreditCapacity(marketId, creditCapacityUsdX18.intoInt256());
}
​
// uint256 -> UD60x18; output default case when market not in Auto Deleverage state
adjustedProfitUsdX18 = ud60x18(profitUsd);
​
// 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 result in wrong adjustedProfitUsdX18 being used in the deposit for the trading account.

Root cause

The getAdjustedProfitForMarketId uses outdated creditCapacityUsdX18 creating a discrepancy between the actual amount that will be withdrawn in the withdrawUsdTokenFromMarket and the amount previewed in getAdjustedProfitForMarketId

Impact

The discrepancy will cause incorrect internal accounting which may even lead to insolvency

Tools Used

Manual Review

Recommendations

Should call recalculateVaultsCreditCapacity prior to previewing the adjustedProfitUsdX18 in _fillOrder

Updates

Lead Judging Commences

inallhonesty Lead Judge 6 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

Should call `recalculateVaultsCreditCapacity` prior to previewing the `adjustedProfitUsdX18` in `_fillOrder`

Appeal created

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

Should call `recalculateVaultsCreditCapacity` prior to previewing the `adjustedProfitUsdX18` in `_fillOrder`

Support

FAQs

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