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

Wrong accounting in the RebalanceVaultsAssets will lead to accounting errors in the vault

bigsam

Summary

While tracking the TOTAL deposited usdc and the marketrealizedebtusd an error was made in utilizing the estimated and not the actual amount returned from the swap action this will lead to an imbalance in tracking the debt system and the accumulation of this can force the contract to become insolvent.

Vulnerability Details

The vault was balanced but the amount used in calculation is not the actual usdc value available hence this lead to wrong account and over inflation of the usdc token amount in the contract.

since we are swapping Vault asset to usdc we should use the returned usdc value.

/// @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.
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[]();
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)
) {
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;
​
// 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
@audit>> vault asset to swap>> 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);
​
@audit>>> returns swapped usdc amount>> dexSwapStrategy.executeSwapExactInputSingle(swapCallData); // return value BUG
​
// SD59x18 -> uint128 using zaros internal precision
@audit>>> uint128 usdDelta = depositAmountUsdX18.intoUint256().toUint128(); // uSE VALUE RETURNEEED BUG BUG BUG
​
// important considerations:
// 1) all subsequent storge updates must use zaros internal precision
// 2) code implicitly assumes that 1 USD = 1 USDC
//
​
@audit >> note // deposits the USDC to the in-credit vault
inCreditVault.depositedUsdc += usdDelta; //BUG
// increase the in-credit vault's share of the markets realized debt
​
//BUG received usdc ==> NOTE // as it has received the USDC and needs to settle it in the future
inCreditVault.marketsRealizedDebtUsd += usdDelta.toInt256().toInt128(); //BUG
​
// ==> NOTE // 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);
}
​

  1. We try to rebalance the vaults as done in other functions

    // get collateral asset amount in native precision of ctx.inDebtVaultCollateralAsset
    @audit>> vault asset to swap>> uint256 assetInputNative = IDexAdapter(ctx.dexAdapter).getExpectedOutput(
    usdc,
    ctx.inDebtVaultCollateralAsset,
    // convert usdc input to native precision
    Collateral.load(usdc).convertSd59x18ToTokenAmount(depositAmountUsdX18)
    );

We confirmed the amount of vault assets needed to get the usdc amounts that we need.

2.We make a call and swap the native to usdc but note due to slippage the amount returned will be less than 1% or more than the targeted usdc amount. NOTE WE USE EXACTIN TO swap using exact out would have been good but with exact in.

The usdc amount is not equal to the targeted 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); // return value BUG
​

3.The swap calls the function below and because of slippage returns a value lesser than the depositamount

/// @inheritdoc IDexAdapter
function executeSwapExactInputSingle(SwapExactInputSinglePayload calldata swapPayload)
external
returns (uint256 amountOut)
{
// transfer the tokenIn from the send to this contract
IERC20(swapPayload.tokenIn).transferFrom(msg.sender, address(this), swapPayload.amountIn);
​
// aprove the tokenIn to the swap router
address uniswapV2SwapStrategyRouterCache = uniswapV2SwapStrategyRouter;
IERC20(swapPayload.tokenIn).approve(uniswapV2SwapStrategyRouterCache, swapPayload.amountIn);
​
// get the expected output amount
@audit>> uint256 expectedAmountOut = getExpectedOutput(swapPayload.tokenIn, swapPayload.tokenOut, swapPayload.amountIn);
​
// Calculate the minimum acceptable output based on the slippage tolerance
@audit>> uint256 amountOutMinimum = calculateAmountOutMin(expectedAmountOut);
​
address[] memory path = new address[]();
path[0] = swapPayload.tokenIn;
path[1] = swapPayload.tokenOut;
​
@audit>> uint256[] memory amountsOut = IUniswapV2Router02(uniswapV2SwapStrategyRouterCache).swapExactTokensForTokens({
amountIn: swapPayload.amountIn,
amountOutMin: amountOutMinimum,
path: path,
to: swapPayload.recipient,
deadline: deadline
});
​
@audit>> return amountsOut[1];
}

4.minout

/// @notice Calculate the amount out min
/// @param amountOutMinExpected The amount out min expected
/// @return amountOutMin The amount out min
function calculateAmountOutMin(uint256 amountOutMinExpected) public view returns (uint256 amountOutMin) {
// calculate the amount out min
amountOutMin =
(amountOutMinExpected * (Constants.BPS_DENOMINATOR - slippageToleranceBps)) / Constants.BPS_DENOMINATOR;
}

Knowing this the amount swapped in will always be lesser but the contract incorrectly used depositamount in it accounting

// SD59x18 -> uint128 using zaros internal precision
@audit>> wrong.. use swap return>> uint128 usdDelta = depositAmountUsdX18.intoUint256().toUint128(); // uSE VALUE RETURNEEED BUG BUG BUG
// 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
​
@audit>> wrong.. use swap return>> inCreditVault.depositedUsdc += usdDelta; //BUG
// 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
​
@audit>> wrong.. use swap return>> inCreditVault.marketsRealizedDebtUsd += usdDelta.toInt256().toInt128(); //BUG
​
// withdraws the USDC from the in-debt vault
​
​
@audit>> wrong.. use swap return>> 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
​
@audit>> wrong.. use swap return>> inDebtVault.marketsRealizedDebtUsd -= usdDelta.toInt256().toInt128();
​
// emit an event
emit LogRebalanceVaultsAssets(vaultsIds[0], vaultsIds[1], usdDelta);
}

Comparing this error to other places where the implementation was correctly handled

  1. settlevaultsdebts function

​
// swap the vault's assets to usdc in order to cover the usd denominated debt partially or fully
// both input and output in native precision
ctx.usdcOut = _convertAssetsToUsdc(
vault.swapStrategy.usdcDexSwapStrategyId,
ctx.vaultAsset,
ctx.swapAmount,
vault.swapStrategy.usdcDexSwapPath,
address(this),
ctx.usdc
);
​
// sanity check to ensure we didn't somehow give away the input tokens
if (ctx.usdcOut == 0) revert Errors.ZeroOutputTokens();
​
// uint256 -> udc60x18 scaling native precision to zaros internal precision
ctx.usdcOutX18 = usdcCollateralConfig.convertTokenAmountToUd60x18(ctx.usdcOut);
​
// use the amount of usdc bought with assets to update the vault's state
// note: storage updates must be done using zaros internal precision
//
// deduct the amount of usdc swapped for assets from the vault's unsettled debt
vault.marketsRealizedDebtUsd -= ctx.usdcOutX18.intoUint256().toInt256().toInt128();
​
// allocate the usdc acquired to back the engine's usd token
UsdTokenSwapConfig.load().usdcAvailableForEngine[vault.engine] += ctx.usdcOutX18.intoUint256();

Impact

Wrong account tracking will lead to multiple accounting error and inaccurate tracking of many sensitive functions like position closing and liquidation.

Tools Used

Manual Review

Recommendations

Use the returned usdc from swap as this will accurately state the actual amount obtained to update the system.

Updates

Lead Judging Commences

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

`CreditDelegationBranch::rebalanceVaultsAssets` doesn't take DEX swap slippage into consideration when swapping debt vault's collateral asset to credit vault's usdc

he rebalanceVaultsAssets function in CreditDelegationBranch.sol updates vault accounting using the pre-swap USD value (usdDelta) rather than the actual post-swap USDC amount received. This means slippage is not accounted for, causing accounting misalignment - if there's negative slippage, the credit vault gets credited more USDC than actually received; if there's positive slippage, it gets credited less.

Support

FAQs

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