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); } }