Summary
In the VaultRouterBranch::deposit function, the Vault::recalculateVaultsCreditCapacity function is called before the deposited collateral is accounted for in the vault's total assets. This results in incorrect credit delegation calculations, causing the depositCreditForMarket function to fail even when the vault has sufficient collateral. This bug disrupts the protocol's core functionality, preventing perp engine from depositing credit into markets.
Vulnerability Details
The vulnerability is located in the VaultRouterBranch::deposit function, specifically in the sequence of operations where Vault::recalculateVaultsCreditCapacity is called before the deposited collateral is added to the vault's total assets. See VaultRouterBranch::deposit:
function deposit(
uint128 vaultId,
uint128 assets,
uint128 minShares,
bytes memory referralCode,
bool isCustomReferralCode
)
external
{
if (assets == 0) revert Errors.ZeroInput("assets");
MarketMakingEngineConfiguration.Data storage marketMakingEngineConfiguration =
MarketMakingEngineConfiguration.load();
address whitelistCache = marketMakingEngineConfiguration.whitelist;
if (whitelistCache != address(0)) {
if (!Whitelist(whitelistCache).verifyIfUserIsAllowed(msg.sender)) {
revert Errors.UserIsNotAllowed(msg.sender);
}
}
Vault.Data storage vault = Vault.loadLive(vaultId);
if (!vault.collateral.isEnabled) revert Errors.VaultDoesNotExist(vaultId);
DepositContext memory ctx;
ctx.vaultAsset = vault.collateral.asset;
uint256[] memory vaultsIds = new uint256[]();
vaultsIds[0] = uint256(vaultId);
Vault.recalculateVaultsCreditCapacity(vaultsIds);
ctx.referralModule = IReferral(marketMakingEngineConfiguration.referralModule);
if (referralCode.length != 0) {
ctx.referralModule.registerReferral(
abi.encode(msg.sender), msg.sender, referralCode, isCustomReferralCode
);
}
ctx.vaultAssetDecimals = vault.collateral.decimals;
ctx.assetsX18 = Math.convertTokenAmountToUd60x18(ctx.vaultAssetDecimals, assets);
ctx.vaultDepositFee = ud60x18(vault.depositFee);
if (ctx.vaultDepositFee.isZero()) {
ctx.assetsMinusFees = assets;
} else {
ctx.assetFeesX18 = ctx.assetsX18.mul(ctx.vaultDepositFee);
ctx.assetFees = Math.convertUd60x18ToTokenAmount(ctx.vaultAssetDecimals, ctx.assetFeesX18);
if (ctx.assetFees == 0) revert Errors.ZeroFeeNotAllowed();
ctx.assetsMinusFees = assets - ctx.assetFees;
if (ctx.assetsMinusFees == 0) revert Errors.DepositTooSmall();
}
IERC20(ctx.vaultAsset).safeTransferFrom(msg.sender, address(this), ctx.assetsMinusFees);
if (ctx.assetFees > 0) {
IERC20(ctx.vaultAsset).safeTransferFrom(
msg.sender, marketMakingEngineConfiguration.vaultDepositAndRedeemFeeRecipient, ctx.assetFees
);
}
address indexTokenCache = vault.indexToken;
IERC20(ctx.vaultAsset).approve(indexTokenCache, ctx.assetsMinusFees);
ctx.shares = IERC4626(indexTokenCache).deposit(ctx.assetsMinusFees, msg.sender);
if (ctx.shares < minShares) revert Errors.SlippageCheckFailed(minShares, ctx.shares);
if (ctx.shares == 0) revert Errors.DepositMustReceiveShares();
emit LogDeposit(vaultId, msg.sender, ctx.assetsMinusFees);
}
The incorrect logic lies in the sequence of operations where in VaultRouterBranch::deposit, Vault::recalculateVaultsCreditCapacity is called before the deposited collateral is transferred to the vault.This means the vault's total assets used in credit capacity calculations including Vault::updateVaultAndCreditDelegationWeight and especially Vault::_updateCreditDelegations where market::totaldelegatedcreditusd is updated do not include the newly deposited collateral.
Vault::recalculateVaultsCreditCapacity function updates the credit delegation for connected markets based on the vault's current total assets (excluding the new deposit).As a result, the Market::totalDelegatedCreditUsd value is not updated to reflect the new deposit, leading to incorrect credit capacity calculations.
CreditDelegation::depositCreditForMarket function checks if Market::totalDelegatedCreditUsd is greater than zero before allowing credit deposits. Due to the incorrect credit delegation calculations, this check fails even when the vault has sufficient collateral, causing the function to revert with Errors.NoDelegatedCredit.
Proof Of Code (POC)
function test\_doswhenuserhasdepositedbutdelegatedmarketcreditis0(
uint256 vaultId,
uint256 marketId
) external {
vm.stopPrank();
VaultConfig memory fuzzVaultConfig = getFuzzVaultConfig(vaultId);
vm.assume(fuzzVaultConfig.asset != address(usdc));
PerpMarketCreditConfig memory fuzzMarketConfig = getFuzzPerpMarketCreditConfig(marketId);
uint256[] memory marketIds = new uint256[](1);
marketIds[0] = fuzzMarketConfig.marketId;
uint256[] memory vaultIds = new uint256[](1);
vaultIds[0] = fuzzVaultConfig.vaultId;
vm.prank(users.owner.account);
marketMakingEngine.connectVaultsAndMarkets(marketIds, vaultIds);
marketMakingEngine.workaround_updateMarketTotalDelegatedCreditUsd(fuzzMarketConfig.marketId, 0);
address user = users.naruto.account;
deal(fuzzVaultConfig.asset, user, 100e18);
vm.startPrank(user);
marketMakingEngine.deposit(fuzzVaultConfig.vaultId, 1e18, 0, "", false);
vm.stopPrank();
vm.prank(address(fuzzMarketConfig.engine));
vm.expectRevert(abi.encodeWithSelector(Errors.NoDelegatedCredit.selector, fuzzMarketConfig.marketId));
marketMakingEngine.depositCreditForMarket(fuzzMarketConfig.marketId, fuzzVaultConfig.asset, 1e17);
}
Impact
Disruption of Core Functionality: Perp Engine cannot deposit credit into markets, even when the vault has sufficient collateral. This prevents the protocol from functioning as intended, disrupting trading and liquidity provision. Traders and liquidity providers may be unable to execute trades or earn rewards, leading to potential financial losses.
Tools Used
Manual Review, Foundry
Recommendations
Update CreditDelegationBranch::depositcreditformarket as follows:
function depositCreditForMarket(
uint128 marketId,
address collateralAddr,
uint256 amount
)
external
onlyRegisteredEngine(marketId)
{
if (amount == 0) revert Errors.ZeroInput("amount");
Collateral.Data storage collateral = Collateral.load(collateralAddr);
collateral.verifyIsEnabled();
Market.Data storage market = Market.loadLive(marketId);
uint256[] memory connectedVaults = market.getConnectedVaultsIds();
Vault.recalculateVaultsCreditCapacity(connectedVaults);
if (market.getTotalDelegatedCreditUsd().isZero()) {
revert Errors.NoDelegatedCredit(marketId);
}
UD60x18 amountX18 = collateral.convertTokenAmountToUd60x18(amount);
address usdToken = MarketMakingEngineConfiguration.load().usdTokenOfEngine[msg.sender];
address usdc = MarketMakingEngineConfiguration.load().usdc;
if (collateralAddr == usdToken) {
market.updateNetUsdTokenIssuance(unary(amountX18.intoSD59x18()));
} else {
if (collateralAddr == usdc) {
market.settleCreditDeposit(address(0), amountX18);
} else {
market.depositCredit(collateralAddr, amountX18);
}
}
IERC20(collateralAddr).safeTransferFrom(msg.sender, address(this), amount);
emit LogDepositCreditForMarket(msg.sender, marketId, collateralAddr, amount);
}
Vault::recalculateVaultsCreditCapacity call ensures that the Market::totalDelegatedCreditUsd value is updated to reflect the latest vault balances. This prevents the NoDelegatedCredit error because the market will have the correct delegated credit value. If this is the first deposit into a vault, Vault::recalculateVaultsCreditCapacity call ensures that the market's delegated credit is updated to reflect the new deposit. This allows the CreditDelegationBranch::depositCreditForMarket function to proceed without reverting.
The recommendation helps maintains consistency across functions. CreditDelegationBranch::withdrawUsdTokenFromMarket function already includes this logic, so adding it to CreditDelegationBranch::depositCreditForMarket ensures consistency across similar operations. This makes the codebase more predictable and easier to maintain. Also, if multiple deposits or withdrawals occur in quick succession, the new solution ensures that the credit delegation state is always up-to-date before processing any credit-related operations. This reduces the risk of race conditions or stale state issues.
