//
The withdrawal of Multi strategies vault could be DoSed while asset deposits remain unaffected
0xpiken profile image0xpiken
Medium

Finding description and impact

The MultiStrategy#_deallocateAssets() function will be DoSed if IStrategy#undeploy(0) is called.

Proof of Concept

When withdrawing user assets from a multi-strategies vault, it will be withdrawn pro-rata from the strategies base on their deployed assets:
https://github.com/code-423n4/2024-12-bakerfi/blob/main/contracts/core/MultiStrategyVault.sol:

function _undeploy(uint256 assets) internal virtual override returns (uint256 undeployedAmount) { undeployedAmount = _deallocateAssets(assets); // Deallocates assets from the strategies and returns the undeployed amount }

https://github.com/code-423n4/2024-12-bakerfi/blob/main/contracts/core/MultiStrategy.sol:

function _deallocateAssets(uint256 amount) internal returns (uint256 totalUndeployed) { uint256[] memory currentAssets = new uint256[](_strategies.length); uint256 totalAssets = 0; uint256 strategiesLength = _strategies.length; for (uint256 i = 0; i < strategiesLength; i++) { currentAssets[i] = IStrategy(_strategies[i]).totalAssets(); totalAssets += currentAssets[i]; } totalUndeployed = 0; for (uint256 i = 0; i < strategiesLength; i++) { uint256 fractAmount = (amount * currentAssets[i]) / totalAssets; totalUndeployed += IStrategy(_strategies[i]).undeploy(fractAmount); } }

If a strategy has not yet deployed any assets, its totalAssets() call will return zero. Subsequently, the system will attempt to execute IStrategy.undeploy(0). However, this call is likely to revert, potentially leading to a denial-of-service condition for the entire withdrawal function:
StrategySupplyBase.sol reverts ZeroAmount() when amount is 0:

function undeploy(uint256 amount) external nonReentrant onlyOwner returns (uint256 undeployedAmount) { @> if (amount == 0) revert ZeroAmount(); // Get Balance uint256 balance = getBalance(); if (amount > balance) revert InsufficientBalance(); // Transfer assets back to caller uint256 withdrawalValue = _undeploy(amount); // Check withdrawal value matches the initial amount // Transfer assets to user ERC20(_asset).safeTransfer(msg.sender, withdrawalValue); balance -= amount; emit StrategyUndeploy(msg.sender, withdrawalValue); emit StrategyAmountUpdate(balance); return amount; }

StrategyLeverage.sol reverts NoCollateralMarginToScale() because totalCollateralInDebt is equal to totalDebtBalance(both of them are 0):

function undeploy(uint256 amount) external onlyOwner nonReentrant returns (uint256 undeployedAmount) { undeployedAmount = _undeploy(amount, payable(msg.sender)); } function _undeploy(uint256 amount, address receiver) private returns (uint256 receivedAmount) { // Get price options from settings IOracle.PriceOptions memory options = IOracle.PriceOptions({ maxAge: getPriceMaxAge(), maxConf: getPriceMaxConf() }); // Fetch collateral and debt balances (uint256 totalCollateralBalance, uint256 totalDebtBalance) = getBalances(); uint256 totalCollateralInDebt = _toDebt(options, totalCollateralBalance, false); // Ensure the position is not in liquidation state @> if (totalCollateralInDebt <= totalDebtBalance) revert NoCollateralMarginToScale(); // Calculate percentage to burn to accommodate the withdrawal uint256 percentageToBurn = (amount * PERCENTAGE_PRECISION) / (totalCollateralInDebt - totalDebtBalance); // Calculate delta position (collateral and debt) (uint256 deltaCollateralInDebt, uint256 deltaDebt) = _calcDeltaPosition( percentageToBurn, totalCollateralInDebt, totalDebtBalance ); // Convert deltaCollateralInDebt to deltaCollateralAmount uint256 deltaCollateralAmount = _toCollateral(options, deltaCollateralInDebt, true); // Calculate flash loan fee uint256 fee = flashLender().flashFee(_debtToken, deltaDebt); // Approve the flash lender to spend the debt amount plus fee if (!IERC20Upgradeable(_debtToken).approve(flashLenderA(), deltaDebt + fee)) { revert FailedToApproveAllowance(); } // Prepare data for flash loan execution bytes memory data = abi.encode(deltaCollateralAmount, receiver, FlashLoanAction.PAY_DEBT_WITHDRAW); _flashLoanArgsHash = keccak256(abi.encodePacked(address(this), _debtToken, deltaDebt, data)); // Execute flash loan if (!flashLender().flashLoan(IERC3156FlashBorrowerUpgradeable(this), _debtToken, deltaDebt, data)) { _flashLoanArgsHash = 0; revert FailedToRunFlashLoan(); } // The amount of Withdrawn minus the repay ampunt emit StrategyUndeploy(msg.sender, deltaCollateralInDebt - deltaDebt); // Reset hash after successful flash loan _flashLoanArgsHash = 0; // Update deployed assets after withdrawal receivedAmount = _pendingAmount; uint256 undeployedAmount = deltaCollateralInDebt - deltaDebt; _deployedAssets = _deployedAssets > undeployedAmount ? _deployedAssets - undeployedAmount : 0; // Emit strategy update and reset pending amount emit StrategyAmountUpdate(_deployedAssets); // Pending amount is not cleared to save gas //_pendingAmount = 0; }

Copy below codes to VaultMultiStrategy.ts and run npm run test:

it.only('Withdraw is DoSed', async () => { const { vault, usdc, owner, park1, park2 } = await loadFixture(deployMultiStrategyVaultFixture); const amount = 10000n * 10n ** 18n; await usdc.approve(vault.target, amount); await vault.deposit(amount, owner.address); //@audit-info deploy a erc4626 vault for StrategySupplyERC4626 const ERC4626Vault = await ethers.getContractFactory("ERC4626VaultMock"); const erc4626Vault = await ERC4626Vault.deploy(usdc.getAddress()); await erc4626Vault.waitForDeployment(); // Deploy StrategySupply contract const StrategySupply = await ethers.getContractFactory('StrategySupplyERC4626'); const strategy = await StrategySupply.deploy( owner.address, await usdc.getAddress(), await erc4626Vault.getAddress(), ); await strategy.waitForDeployment(); await strategy.transferOwnership(await vault.getAddress()); //@audit-info add the new strategy to vault await vault.addStrategy(await strategy.getAddress()); //@audit-info the new strategy has been added expect(await vault.strategies()).to.include(await strategy.getAddress()); //@audit-info withdrawal is DoSed await expect(vault.withdraw(amount, owner.address, owner.address)).to.be.revertedWithCustomError( strategy, 'ZeroAmount', ); });

Recommended mitigation steps

Check if the amount is 0 before undeploying it:

function _deallocateAssets(uint256 amount) internal returns (uint256 totalUndeployed) { uint256[] memory currentAssets = new uint256[](_strategies.length); uint256 totalAssets = 0; uint256 strategiesLength = _strategies.length; for (uint256 i = 0; i < strategiesLength; i++) { currentAssets[i] = IStrategy(_strategies[i]).totalAssets(); totalAssets += currentAssets[i]; } totalUndeployed = 0; for (uint256 i = 0; i < strategiesLength; i++) { uint256 fractAmount = (amount * currentAssets[i]) / totalAssets; + if (fractAmount == 0) continue; totalUndeployed += IStrategy(_strategies[i]).undeploy(fractAmount); } }

Links to affected code