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

The line 629 in the `rebalanceVaultsAssets` function is incorrect.

kupiasec

Summary

The rebalanceVaultsAssets::L629 is incorrect.

Vulnerability Details

https://github.com/Cyfrin/2025-01-zaros-part-2/blob/main/src/market-making/branches/CreditDelegationBranch.sol#L593

/// @notice Rebalances credit and debt between two vaults.
/// @dev There are multiple factors that may result on vaults backing the same engine having a completely
/// different credit or debt state, such as:
/// - connecting vaults with markets in different times
/// - connecting vaults with different sets of markets
/// - users swapping the engine's usd token for assets of different vaults
/// This way, from time to time, the system keepers must rebalance vaults with a significant state difference in
/// order to facilitate settlement of their credit and debt. A rebalancing doesn't need to always fully settle the
/// amount of USDC that a vault in credit requires to settle its due amount, so the system is optimized to ensure
/// a financial stability of the protocol.
/// @dev Example:
/// in credit vault markets realized debt = -100 -> -90
/// in credit vault deposited usdc = 200 -> 210
/// in credit vault unsettled realized debt = -300 | as -100 + -200 -> after settlement -> -300 | as -90 + -210
/// = -300
​
/// thus, we need to rebalance as the in credit vault doesn't own enough usdc to settle its due credit
​
/// in debt vault markets realized debt = 50 -> 40
/// in debt vault deposited usdc = 10 -> 0
/// in debt vault unsettled realized debt = 40 | as 50 + -10 -> after settlement -> 40 | as 40 + 0 = 40
/// @dev The first vault id passed is assumed to be the in credit vault, and the second vault id is assumed to be
/// the in debt vault.
/// @dev The final unsettled realized debt of both vaults MUST remain the same after the rebalance.
/// @dev The actual increase or decrease in the vaults' unsettled realized debt happen at `settleVaultsDebt`.
/// @param vaultsIds The vaults' identifiers to rebalance.
593:function rebalanceVaultsAssets(uint128[2] calldata vaultsIds) external onlyRegisteredSystemKeepers {
// load the storage pointers of the vaults in net credit and net debt
Vault.Data storage inCreditVault = Vault.loadExisting(vaultsIds[0]);
Vault.Data storage inDebtVault = Vault.loadExisting(vaultsIds[1]);
​
// both vaults must belong to the same engine in order to have their debt
// state rebalanced, as each usd token's debt is isolated
if (inCreditVault.engine != inDebtVault.engine) {
revert Errors.VaultsConnectedToDifferentEngines();
}
​
// create an in-memory dynamic array in order to call `Vault::recalculateVaultsCreditCapacity`
uint256[] memory vaultsIdsForRecalculation = new uint256[](2);
vaultsIdsForRecalculation[0] = vaultsIds[0];
vaultsIdsForRecalculation[1] = vaultsIds[1];
​
// recalculate the credit capacity of both vaults
Vault.recalculateVaultsCreditCapacity(vaultsIdsForRecalculation);
​
// cache the in debt vault & in credit vault unsettled debt
SD59x18 inDebtVaultUnsettledRealizedDebtUsdX18 = inDebtVault.getUnsettledRealizedDebt();
SD59x18 inCreditVaultUnsettledRealizedDebtUsdX18 = inCreditVault.getUnsettledRealizedDebt();
​
// revert if 1) the vault that is supposed to be in credit is not OR
// 2) the vault that is supposed to be in debt is not
if (
619: inCreditVaultUnsettledRealizedDebtUsdX18.lte(SD59x18_ZERO)
|| inDebtVaultUnsettledRealizedDebtUsdX18.gte(SD59x18_ZERO)
) {
revert Errors.InvalidVaultDebtSettlementRequest();
}
​
// get debt absolute value
SD59x18 inDebtVaultUnsettledRealizedDebtUsdX18Abs = inDebtVaultUnsettledRealizedDebtUsdX18.abs();
​
// if debt absolute value > credit, use credit value, else use debt value
629: SD59x18 depositAmountUsdX18 = inCreditVaultUnsettledRealizedDebtUsdX18.gt(
inDebtVaultUnsettledRealizedDebtUsdX18Abs
) ? inDebtVaultUnsettledRealizedDebtUsdX18Abs : inCreditVaultUnsettledRealizedDebtUsdX18;
​
// loads the dex swap strategy data storage pointer
DexSwapStrategy.Data storage dexSwapStrategy =
DexSwapStrategy.loadExisting(inDebtVault.swapStrategy.usdcDexSwapStrategyId);
​
// load usdc address
address usdc = MarketMakingEngineConfiguration.load().usdc;
​
// cache input asset and dex adapter
CalculateSwapContext memory ctx;
ctx.inDebtVaultCollateralAsset = inDebtVault.collateral.asset;
ctx.dexAdapter = dexSwapStrategy.dexAdapter;
​
// get collateral asset amount in native precision of ctx.inDebtVaultCollateralAsset
uint256 assetInputNative = IDexAdapter(ctx.dexAdapter).getExpectedOutput(
usdc,
ctx.inDebtVaultCollateralAsset,
// convert usdc input to native precision
650: Collateral.load(usdc).convertSd59x18ToTokenAmount(depositAmountUsdX18)
);
​
// prepare the data for executing the swap asset -> usdc
SwapExactInputSinglePayload memory swapCallData = SwapExactInputSinglePayload({
tokenIn: ctx.inDebtVaultCollateralAsset,
tokenOut: usdc,
amountIn: assetInputNative,
recipient: address(this) // deposit the usdc to the market making engine proxy
});
​
// approve the collateral token to the dex adapter and swap assets for USDC
IERC20(ctx.inDebtVaultCollateralAsset).approve(ctx.dexAdapter, assetInputNative);
663: dexSwapStrategy.executeSwapExactInputSingle(swapCallData);
​
// SD59x18 -> uint128 using zaros internal precision
uint128 usdDelta = depositAmountUsdX18.intoUint256().toUint128();
​
// important considerations:
// 1) all subsequent storge updates must use zaros internal precision
// 2) code implicitly assumes that 1 USD = 1 USDC
//
// deposits the USDC to the in-credit vault
inCreditVault.depositedUsdc += usdDelta;
// increase the in-credit vault's share of the markets realized debt
// as it has received the USDC and needs to settle it in the future
inCreditVault.marketsRealizedDebtUsd += usdDelta.toInt256().toInt128();
​
// withdraws the USDC from the in-debt vault
679: inDebtVault.depositedUsdc -= usdDelta;
// decrease the in-debt vault's share of the markets realized debt
// as it has transferred USDC to the in-credit vault
inDebtVault.marketsRealizedDebtUsd -= usdDelta.toInt256().toInt128();
​
// emit an event
emit LogRebalanceVaultsAssets(vaultsIds[0], vaultsIds[1], usdDelta);
}
Math.sol
function convertSd59x18ToTokenAmount(uint8 decimals, SD59x18 amountX18) internal pure returns (uint256) {
if (Constants.SYSTEM_DECIMALS == decimals) {
84: return amountX18.intoUint256();
}
​
return amountX18.intoUint256() / (10 ** (Constants.SYSTEM_DECIMALS - decimals));
}

In L629, depositAmountUsd is min(UnsettledRealizedDebt in DebtVault, UnsettledRealizedDebtUsdAbs in CreditVault), instead of
min(UnsettledRealizedDebtUsdAbs in DebtVault, depositedUsdc in DebtVault).
As a result, due to L650 or L679, this function could be reverted.

Let's consider the senario in the code comment.
In credit vault, markets realized debt = -100
In credit vault, deposited usdc = 200
In debt vault, markets realized debt = 50
In debt vault, deposited usdc = 10

inCreditVaultUnsettledRealizedDebtUsd = -300, inDebtVaultUnsettledRealizedDebtUsd = 40

In L619, the condition inCreditVaultUnsettledRealizedDebtUsdX18 = -300 <= 0 causes the call to revert.
Thus, we assume that the credit vault and debt vault is swaped.

  1. If the L619 check is passed:
    In L629, the depositAmountUsd = min(40, -300) = -300.
    In L650, this transaction is reverted.

  2. If the creditVault and debtVault are swappped:
    In L629, the depositAmountUsd = min(40, 300) = 40.
    However, in L679, due to the L679(10 -= 40) check, this transaction is reverted.

Impact

Markets don't work as intended.
The rebalanceVaultsAssets function could revert when it shouln't.

Recommendations

593:function rebalanceVaultsAssets(uint128[2] calldata vaultsIds) external onlyRegisteredSystemKeepers {
// load the storage pointers of the vaults in net credit and net debt
Vault.Data storage inCreditVault = Vault.loadExisting(vaultsIds[0]);
Vault.Data storage inDebtVault = Vault.loadExisting(vaultsIds[1]);
​
// both vaults must belong to the same engine in order to have their debt
// state rebalanced, as each usd token's debt is isolated
if (inCreditVault.engine != inDebtVault.engine) {
revert Errors.VaultsConnectedToDifferentEngines();
}
​
// create an in-memory dynamic array in order to call `Vault::recalculateVaultsCreditCapacity`
uint256[] memory vaultsIdsForRecalculation = new uint256[](2);
vaultsIdsForRecalculation[0] = vaultsIds[0];
vaultsIdsForRecalculation[1] = vaultsIds[1];
​
// recalculate the credit capacity of both vaults
Vault.recalculateVaultsCreditCapacity(vaultsIdsForRecalculation);
​
// cache the in debt vault & in credit vault unsettled debt
SD59x18 inDebtVaultUnsettledRealizedDebtUsdX18 = inDebtVault.getUnsettledRealizedDebt();
SD59x18 inCreditVaultUnsettledRealizedDebtUsdX18 = inCreditVault.getUnsettledRealizedDebt();
​
// revert if 1) the vault that is supposed to be in credit is not OR
// 2) the vault that is supposed to be in debt is not
if (
- inCreditVaultUnsettledRealizedDebtUsdX18.lte(SD59x18_ZERO)
- || inDebtVaultUnsettledRealizedDebtUsdX18.gte(SD59x18_ZERO)
+ inCreditVaultUnsettledRealizedDebtUsdX18.gte(SD59x18_ZERO)
+ || inDebtVaultUnsettledRealizedDebtUsdX18.lte(SD59x18_ZERO)
​
) {
revert Errors.InvalidVaultDebtSettlementRequest();
}
​
// get debt absolute value
SD59x18 inDebtVaultUnsettledRealizedDebtUsdX18Abs = inDebtVaultUnsettledRealizedDebtUsdX18.abs();
​
// if debt absolute value > credit, use credit value, else use debt value
- SD59x18 depositAmountUsdX18 = inCreditVaultUnsettledRealizedDebtUsdX18.gt(
- inDebtVaultUnsettledRealizedDebtUsdX18Abs
- ) ? inDebtVaultUnsettledRealizedDebtUsdX18Abs : inCreditVaultUnsettledRealizedDebtUsdX18;
+ SD59x18 depositAmountUsdX18 = (inDebtVault.marketsRealizedDebtUsd).gt(
+ inDebtVault.depositedUsdc
+ ) ? inDebtVault.depositedUsdc : inDebtVault.marketsRealizedDebtUsd;
​
// loads the dex swap strategy data storage pointer
DexSwapStrategy.Data storage dexSwapStrategy =
DexSwapStrategy.loadExisting(inDebtVault.swapStrategy.usdcDexSwapStrategyId);
​
// load usdc address
address usdc = MarketMakingEngineConfiguration.load().usdc;
​
// cache input asset and dex adapter
CalculateSwapContext memory ctx;
ctx.inDebtVaultCollateralAsset = inDebtVault.collateral.asset;
ctx.dexAdapter = dexSwapStrategy.dexAdapter;
​
// get collateral asset amount in native precision of ctx.inDebtVaultCollateralAsset
uint256 assetInputNative = IDexAdapter(ctx.dexAdapter).getExpectedOutput(
usdc,
ctx.inDebtVaultCollateralAsset,
// convert usdc input to native precision
Collateral.load(usdc).convertSd59x18ToTokenAmount(depositAmountUsdX18)
);
​
// prepare the data for executing the swap asset -> usdc
SwapExactInputSinglePayload memory swapCallData = SwapExactInputSinglePayload({
tokenIn: ctx.inDebtVaultCollateralAsset,
tokenOut: usdc,
amountIn: assetInputNative,
recipient: address(this) // deposit the usdc to the market making engine proxy
});
​
// approve the collateral token to the dex adapter and swap assets for USDC
IERC20(ctx.inDebtVaultCollateralAsset).approve(ctx.dexAdapter, assetInputNative);
dexSwapStrategy.executeSwapExactInputSingle(swapCallData);
​
// SD59x18 -> uint128 using zaros internal precision
uint128 usdDelta = depositAmountUsdX18.intoUint256().toUint128();
​
// important considerations:
// 1) all subsequent storge updates must use zaros internal precision
// 2) code implicitly assumes that 1 USD = 1 USDC
//
// deposits the USDC to the in-credit vault
inCreditVault.depositedUsdc += usdDelta;
// increase the in-credit vault's share of the markets realized debt
// as it has received the USDC and needs to settle it in the future
inCreditVault.marketsRealizedDebtUsd += usdDelta.toInt256().toInt128();
​
// withdraws the USDC from the in-debt vault
inDebtVault.depositedUsdc -= usdDelta;
// decrease the in-debt vault's share of the markets realized debt
// as it has transferred USDC to the in-credit vault
inDebtVault.marketsRealizedDebtUsd -= usdDelta.toInt256().toInt128();
​
// emit an event
emit LogRebalanceVaultsAssets(vaultsIds[0], vaultsIds[1], usdDelta);
}
Updates

Lead Judging Commences

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

depositAmountUsdX18 calculation inside CreditDelegatioNBranch::rebalanceVaultsAssets is wrong

Support

FAQs

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