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

`Market::getRealizedDebtUsd` calculates realized debt incorrectly

alexczm

Summary

The realized debt returned by getRealizedDebtUsd is incorrectly calculated. This information is used all over the project, breaking the countability

Vulnerability Details

Upon closing a profitable position trading engine calls creditDelegationBranch::withdrawUsdTokenFromMarket to reward the trader.
This function will mint the required usdToken and increase the market's netUsdTokenIssuance by the same amount.

// CreditDelegationBranch.sol
function withdrawUsdTokenFromMarket(uint128 marketId, uint256 amount) external onlyRegisteredEngine(marketId) {
...
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);
...

The netUsdTokenIssuance represents the net amount of usdToken minted and burned by respective market. This amount is seen as a debt for the market.

On the other hand when a trader is liquidated (or settles a position with negative PnL) collateral is transferred from trading engine to market-making engine via CreditDelegationBranch::depositCreditForMarket.

//CreditDelegationBranch.sol
​
function depositCreditForMarket(
uint128 marketId,
address collateralAddr,
uint256 amount
)
external
onlyRegisteredEngine(marketId)
{
if (collateralAddr == usdToken) {
// if the deposited collateral is USD Token, it reduces the market's realized debt
@> market.updateNetUsdTokenIssuance(unary(amountX18.intoSD59x18()));
} else {
if (collateralAddr == usdc) {
market.settleCreditDeposit(address(0), amountX18);
} else {
// deposits the received collateral to the market to be distributed to vaults
// to be settled in the future
@> market.depositCredit(collateralAddr, amountX18);
}
}
​
// transfers the margin collateral asset from the registered engine to the market making engine
// NOTE: The engine must approve the market making engine to transfer the margin collateral asset, see
// PerpsEngineConfigurationBranch::setMarketMakingEngineAllowance
// note: transfers must occur using token native precision
IERC20(collateralAddr).safeTransferFrom(msg.sender, address(this), amount);

If collateral used is usdToken the market's netUsdTokenIssuance is decreased (see the unary operation), reducing the market's realized debt.
If collateral is USDC, the amount of usdc to be distributed to vaults is updated and
if it's another collateral, it's added to creditDeposits. These deposits represents the market's profit.

//Market.sol
function depositCredit(Data storage self, address asset, UD60x18 amountX18) internal { // 1
AssetToAmountMap.update(self.creditDeposits, asset, amountX18, true);
}

The problem is that Market::getRealizedDebtUsd returns the sum of creditDeposits and netUsdTokenIssuance as realized debt.
Instead it should subtract credit deposits from netUsdTokenIssuance.

function getRealizedDebtUsd(Data storage self) internal view returns (SD59x18 realizedDebtUsdX18) {
// prepare the credit deposits usd value variable;
UD60x18 creditDepositsValueUsdX18;
​
// if the credit deposits usd value cache is up to date, return the stored value
if (block.timestamp <= self.lastCreditDepositsValueRehydration) {
creditDepositsValueUsdX18 = ud60x18(self.creditDepositsValueCacheUsd);
} else {
// otherwise, we'll need to loop over credit deposits to calculate it
@> creditDepositsValueUsdX18 = getCreditDepositsValueUsd(self); // @audit get credit deposits value adjusted by creditRatio
}
​
// finally after determining the market's latest credit deposits usd value, sum it with the stored net usd
// token issuance to return the net realized debt usd value
@> realizedDebtUsdX18 = creditDepositsValueUsdX18.intoSD59x18().add(sd59x18(self.netUsdTokenIssuance)); // @audit it should be debtValue - creditValue
}

The returned value is used many places

Impact

The getCreditDepositsValueUsd returned values is used in many places in the codebase, breaking the market-making engine accounting.

Tools Used

Recommendations

function getRealizedDebtUsd(Data storage self) internal view returns (SD59x18 realizedDebtUsdX18) {
// prepare the credit deposits usd value variable;
UD60x18 creditDepositsValueUsdX18;
​
// if the credit deposits usd value cache is up to date, return the stored value
if (block.timestamp <= self.lastCreditDepositsValueRehydration) {
creditDepositsValueUsdX18 = ud60x18(self.creditDepositsValueCacheUsd);
} else {
// otherwise, we'll need to loop over credit deposits to calculate it
creditDepositsValueUsdX18 = getCreditDepositsValueUsd(self);
}
​
// finally after determining the market's latest credit deposits usd value, sum it with the stored net usd
// token issuance to return the net realized debt usd value
- realizedDebtUsdX18 = creditDepositsValueUsdX18.intoSD59x18().add(sd59x18(self.netUsdTokenIssuance));
+ realizedDebtUsdX18 = sd59x18(self.netUsdTokenIssuance).sub(creditDepositsValueUsdX18.intoSD59x18());
}
​
Updates

Lead Judging Commences

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

Market::getRealizedDebtUsd incorrectly adds creditDeposits to netUsdTokenIssuance when calculating debt, causing accounting errors because credit deposits should reduce debt

Support

FAQs

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