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

rebalanceVaultsAssets incorrectly accounts vaults' depositedUsdc

glightspeed2

Summary

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

Vulnerability Details

CreditDelegationBranch.rebalanceVaultsAssets rebalances credit and debt between two vaults by swapping debt vault's collateral asset to USDC and accumulates this to credit vault's depositedUsdc and marketRealizedDebtUsd.

uint256 assetInputNative = IDexAdapter(ctx.dexAdapter).getExpectedOutput(
usdc,
ctx.inDebtVaultCollateralAsset,
Collateral.load(usdc).convertSd59x18ToTokenAmount(depositAmountUsdX18)
);
​
SwapExactInputSinglePayload memory swapCallData = SwapExactInputSinglePayload({
tokenIn: ctx.inDebtVaultCollateralAsset,
tokenOut: usdc,
amountIn: assetInputNative,
recipient: address(this)
});
​
IERC20(ctx.inDebtVaultCollateralAsset).approve(ctx.dexAdapter, assetInputNative);
dexSwapStrategy.executeSwapExactInputSingle(swapCallData); // <-- amountOut
​
uint128 usdDelta = depositAmountUsdX18.intoUint256().toUint128();
​
inCreditVault.depositedUsdc += usdDelta; // @audit usdDelta != amountOut
inCreditVault.marketsRealizedDebtUsd += usdDelta.toInt256().toInt128();
inDebtVault.depositedUsdc -= usdDelta;
inDebtVault.marketsRealizedDebtUsd -= usdDelta.toInt256().toInt128();

The problem is, this swapping is done via DEX like Curve, Uniswap V2/3 that uses CFMM to decide amountOut, while assetInputNative is estimated by price ratio.

So tokenOut will be different than usdDelta by up to slippageToleranceBps%

According to the current implementation, total sum of marketRealizedDebtUsdand total depositedUsdc remains the same after the rebalancing. Thus, slippage is not accounted anywhere.

Impact

  • The protocol will suffer from DEX swap slippage

Tools Used

Manual Review

Recommendations

Consider the following change:

diff --git a/src/market-making/branches/CreditDelegationBranch.sol b/src/market-making/branches/CreditDelegationBranch.sol
index d091d5c..d948a0d 100644
--- a/src/market-making/branches/CreditDelegationBranch.sol
+++ b/src/market-making/branches/CreditDelegationBranch.sol
@@ -660,7 +660,7 @@ contract CreditDelegationBranch is EngineAccessControl {
// approve the collateral token to the dex adapter and swap assets for USDC
IERC20(ctx.inDebtVaultCollateralAsset).approve(ctx.dexAdapter, assetInputNative);
- dexSwapStrategy.executeSwapExactInputSingle(swapCallData);
+ uint128 amountOut = uint128(dexSwapStrategy.executeSwapExactInputSingle(swapCallData));
// SD59x18 -> uint128 using zaros internal precision
uint128 usdDelta = depositAmountUsdX18.intoUint256().toUint128();
@@ -670,13 +670,13 @@ contract CreditDelegationBranch is EngineAccessControl {
// 2) code implicitly assumes that 1 USD = 1 USDC
//
// deposits the USDC to the in-credit vault
- inCreditVault.depositedUsdc += usdDelta;
+ inCreditVault.depositedUsdc += amountOut;
// 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();
​

Rationale behind the Fix

  • Why marketsRealizedDebtUsd is not changed by amountOut?

    • Because it's just transferring debt from one vault to another

  • Why inDebtVault.depositedUsdcis not changed by amountOut?

    • With this fix, after rebalancing, sum of the two vaults' debt remains the same, while sum of depositedUsdc is changed by usdDelta - amountOut

      • Thus, sum of vaults' totalDebt is changed by usdDelta - amountOut

      • This effectively delegates slippage to credit vault.

    • If inDebtVault.depositedUsdcis also changed by amountOut, total depositedUsdc won't be changed, thus silently dropping sloppage

Updates

Lead Judging Commences

inallhonesty Lead Judge 5 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.