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

`depositCreditForMarket` should not increase the debt when `depositCredit` is called

novaman33

Summary

depositCreditForMarket is called during negative PnL adjustments of a position in order to deposit a portion of a user's margin as credit to a specific market. However when the asset is not usd or USDC it will actually increase debt, instead of reducing it. Debt will be decreased after a keeper calls convertMarketsCreditDepositsToUsdc, however this will still create a window in which users will be able to deposit and receive significantly more index tokens.

Vulnerability Details

When a position has a negative PnL, the PerpEngine will call depositCreditForMarket. This will remove collateral from the positions margin and transfer it to the market. However when the collateral is not the USD token of the MarketMakingEngine and is not USDC, the depositCreditForMarket will perform the following call:

function depositCreditForMarket(
uint128 marketId,
address collateralAddr,
uint256 amount
)
external
onlyRegisteredEngine(marketId)
{
...
} 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);
​
// emit an event
emit LogDepositCreditForMarket(msg.sender, marketId, collateralAddr, amount);
}

When credit is deposited, it is added to the creditDeposits mapping.
However when we look into the getRealizedDebtUsd function we see that these creditDeposits are counted towards the debt:

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));
}

Due to this mistake, debt will be overestimated allowing user's to deposit to vauts connected to this market using a wrong rate.
This is because getVaultAssetSwapRate will use the following formula:

function getVaultAssetSwapRate(
uint128 vaultId,
uint256 assetsIn,
bool shouldDiscountDepositFee
)
public
view
returns (UD60x18 sharesOut)
{
// fetch storage slot for vault by id
Vault.Data storage vault = Vault.loadExisting(vaultId);
​
// get the vault's net credit capacity, i.e its total assets usd value minus its total debt (or adding its
// credit if debt is negative)
>>> uint256 totalAssetsMinusVaultDebt = getVaultCreditCapacity(vaultId);
​
// get decimal offset
uint8 decimalOffset = Constants.SYSTEM_DECIMALS - IERC20Metadata(vault.indexToken).decimals();
​
// Get the shares amount out for the input amount of tokens, taking into account the unsettled debt
// See {IERC4626-previewDeposit}.
// `IERC4626(vault.indexToken).totalSupply() + 10 ** decimalOffset` could lead to problems
uint256 previewSharesOut = assetsIn.mulDiv(
IERC4626(vault.indexToken).totalSupply() + 10 ** decimalOffset,
totalAssetsMinusVaultDebt,
MathOpenZeppelin.Rounding.Floor
);

Here totalAssetsMinusVaultDebt will be decreased because of the wrong implementation of the getRealizedDebtUsd which will allow users to get more index tokens.
The users will instantly profit when the keeper calls convertMarketsCreditDepositsToUsdc which will decrease the debt, resulting in the index token being more expensive.

Root cause

Deposited credit is counted as debt, while it should be counted as credit.

Impact

Wrong debt calculation will allow users to get more index tokens at a cheaper price. Also this could also trigger keepers to unnecessarly execute functions such as settleVaultsDebt which will further decrease the vault's credit capacity.

Tools Used

Manual Review

Recommendations

Deposited collateral should be counted against the debt.

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.