Part 2

Summary

After the first call of VaultRouterBranch.sol:redeem() value of newCreditDelegationUsdX18 is less than previousCreditDelegationUsdX18 in Vault.sol:_updateCreditDelegations(), this causes revert due to underflow in functions where Vault.sol:recalculateVaultsCreditCapacity() is called.

Vulnerability Details

VaultRouterBranch.sol:redeem() function calls Vault.sol:recalculateVaultsCreditCapacity() to recalculate vault credit capacity that includes function to update credit delegation - Vault.sol:_updateCreditDelegations():

This function will revert after first redeem, because newCreditDelegationUsdX18 will now always be less than previousCreditDelegationUsdX18:

PoC

1. (Optional) Change src/market-making/leaves/Vault.sol:_updateCreditDelegations(): to emit new events to check values of newCreditDelegationUsdX18 and previousCreditDelegationUsdX18:

   event NewCreditDelegation(UD60x18 value);

   event PreviousCreditDelegation(UD60x18 value);

   ​

   /// @notice Updates the vault's credit delegations to its connected markets, using the provided cache of connected

   /// markets ids.

   /// @dev This function assumes that the connected markets ids cache is up to date with the stored markets ids. If

   /// this invariant resolves to false, the function will not work as expected.

   /// @dev We assume self.totalCreditDelegationWeight is always greater than zero, as it's verified during

   /// configuration.

   /// @param self The vault storage pointer.

   /// @param connectedMarketsIdsCache The cached connected markets ids.

   /// @param shouldRehydrateCache Whether the connected markets ids cache should be rehydrated or not.

   /// @return rehydratedConnectedMarketsIdsCache The potentially rehydrated connected markets ids cache.

   function _updateCreditDelegations(

   Data storage self,

   uint128[] memory connectedMarketsIdsCache,

   bool shouldRehydrateCache

   )

   private

   returns (uint128[] memory rehydratedConnectedMarketsIdsCache, SD59x18 vaultCreditCapacityUsdX18)

   {

   rehydratedConnectedMarketsIdsCache = new uint128[](connectedMarketsIdsCache.length);

   // cache the vault id

   uint128 vaultId = self.id;

   ​

   // cache the connected markets length

   uint256 connectedMarketsConfigLength = self.connectedMarkets.length;

   ​

   // loads the connected markets storage pointer by taking the last configured market ids uint set

   EnumerableSet.UintSet storage connectedMarkets = self.connectedMarkets[connectedMarketsConfigLength - 1];

   ​

   // loop over each connected market id that has been cached once again in order to update this vault's

   // credit delegations

   for (uint256 i; i < connectedMarketsIdsCache.length; i++) {

   // rehydrate the markets ids cache if needed

   if (shouldRehydrateCache) {

   rehydratedConnectedMarketsIdsCache[i] = connectedMarkets.at(i).toUint128();

   } else {

   rehydratedConnectedMarketsIdsCache[i] = connectedMarketsIdsCache[i];

   }

   ​

   // loads the memory cached market id

   uint128 connectedMarketId = rehydratedConnectedMarketsIdsCache[i];

   ​

   // load the credit delegation to the given market id

   CreditDelegation.Data storage creditDelegation = CreditDelegation.load(vaultId, connectedMarketId);

   ​

   // cache the previous credit delegation value

   UD60x18 previousCreditDelegationUsdX18 = ud60x18(creditDelegation.valueUsd);

   ​

   // cache the latest credit delegation share of the vault's credit capacity

   uint128 totalCreditDelegationWeightCache = self.totalCreditDelegationWeight;

   ​

   if (totalCreditDelegationWeightCache != 0) {

   // get the latest credit delegation share of the vault's credit capacity

   UD60x18 creditDelegationShareX18 =

   ud60x18(creditDelegation.weight).div(ud60x18(totalCreditDelegationWeightCache));

   ​

   // stores the vault's total credit capacity to be returned

   vaultCreditCapacityUsdX18 = getTotalCreditCapacityUsd(self);

   ​

   // if the vault's credit capacity went to zero or below, we set its credit delegation to that market

   // to zero

   UD60x18 newCreditDelegationUsdX18 = vaultCreditCapacityUsdX18.gt(SD59x18_ZERO)

   ? vaultCreditCapacityUsdX18.intoUD60x18().mul(creditDelegationShareX18)

   : UD60x18_ZERO;

   ​

   emit NewCreditDelegation(newCreditDelegationUsdX18);

   emit PreviousCreditDelegation(previousCreditDelegationUsdX18);

   // calculate the delta applied to the market's total delegated credit

   UD60x18 creditDeltaUsdX18 = newCreditDelegationUsdX18.sub(previousCreditDelegationUsdX18);

   ​

   // loads the market's storage pointer and update total delegated credit

   Market.Data storage market = Market.load(connectedMarketId);

   market.updateTotalDelegatedCredit(creditDeltaUsdX18);

   ​

   // if new credit delegation is zero, we clear the credit delegation storage

   if (newCreditDelegationUsdX18.isZero()) {

   creditDelegation.clear();

   } else {

   // update the credit delegation stored usd value

   creditDelegation.valueUsd = newCreditDelegationUsdX18.intoUint128();

   }

   }

   }

   }

2. Add this test to test/integration/market-making/vault-router-branch/redeem/redeem.t.sol:

   function test_RevertCreditDelegationUnderflow(

   uint128 vaultId,

   uint128 assetsToDeposit,

   uint128 sharesToWithdraw

   )

   external

   {

   // ensure valid vault and load vault config

   vaultId = uint128(bound(vaultId, INITIAL_VAULT_ID, FINAL_VAULT_ID));

   VaultConfig memory fuzzVaultConfig = getFuzzVaultConfig(vaultId);

   ​

   address user = users.naruto.account;

   address user2 = users.sasuke.account;

   assetsToDeposit = uint128(fuzzVaultConfig.depositCap);

   address vaultAsset = marketMakingEngine.workaround_Vault_getVaultAsset(vaultId);

   ​

   deal(vaultAsset, user, assetsToDeposit);

   deal(vaultAsset, user2, assetsToDeposit);

   ​

   //create two equal deposits for users

   vm.startPrank(user2);

   marketMakingEngine.deposit(vaultId, assetsToDeposit / 2, 0, "", false);

   vm.stopPrank();

   ​

   vm.startPrank(user);

   marketMakingEngine.deposit(vaultId, assetsToDeposit / 2, 0, "", false);

   vm.stopPrank();

   ​

   uint128 user2VaultShares = uint128(IERC20(fuzzVaultConfig.indexToken).balanceOf(user2));

   uint128 userVaultShares = uint128(IERC20(fuzzVaultConfig.indexToken).balanceOf(user));

   //used to show that we try to withdraw only small part of shares

   uint128 partOfSharesToWithdraw = 10;

   ​

   //user2 - successfull redeem

   vm.startPrank(user2);

   marketMakingEngine.initiateWithdrawal(vaultId, user2VaultShares / partOfSharesToWithdraw);

   skip(fuzzVaultConfig.withdrawalDelay + 1);

   marketMakingEngine.redeem(vaultId, WITHDRAW_REQUEST_ID, 0);

   vm.stopPrank();

   ​

   //user1 - try to deposit more - revert due to underflow

   vm.startPrank(user);

   vm.expectRevert();

   marketMakingEngine.deposit(vaultId, assetsToDeposit / 2, 0, "", false);

   vm.stopPrank();

   ​

   //user1 - try to redeem his tokens - revert due to underflow

   vm.startPrank(user);

   sharesToWithdraw = uint128(userVaultShares / partOfSharesToWithdraw);

   marketMakingEngine.initiateWithdrawal(vaultId, sharesToWithdraw);

   skip(fuzzVaultConfig.withdrawalDelay + 1);

   vm.expectRevert();

   marketMakingEngine.redeem(vaultId, WITHDRAW_REQUEST_ID, 0);

   vm.stopPrank();

   }

3. Run this test with command:

Output:

As you can see in NewCreditDelegation and PreviousCreditDelegation events newCreditDelegationUsdX18 is less than previousCreditDelegationUsdX18, causing revert.

Impact

Functions where Vault.sol:recalculateVaultsCreditCapacity()is called will revert after first redeem, this includes functions like VaultRouterBranch.sol:deposit() and VaultRouterBranch.sol:redeem() too.

Tools Used

Manual Review

Recommendations

creditDeltaUsdX18 calculation should consider that newCreditDelegationUsdX18 may be less than previousCreditDelegationUsdX18.