Unhandled code path in WithdrawalQueueHelper._withdrawStrategyFunds
could result in users receiving less assets than they are entitled to
Description
During a user withdrawal from a Concrete Vault, if the withdrawable amount is sufficient for the requested withdrawal, execution proceeds to the WithdrawalQueueHelper._withdrawStrategyFunds
function:
function processWithdrawal(
uint256 assets_,
address receiver_,
uint256 availableAssets,
address asset,
address withdrawalQueue,
uint256 minQueueRequest,
Strategy[] memory strategies,
IParkingLot parkingLot
) external {
if (availableAssets >= assets_) {
_withdrawStrategyFunds(assets_, receiver_, asset, strategies, parkingLot);
} else {
// [...]
}
}
The _withdrawStrategyFunds
function first checks the asset balance in the vault contract (float
). If the balance is sufficient for the withdrawal, the function withdraws funds from the balance. Otherwise, the vault withdraws proportional amounts from the strategies to the user based on the strategies' allocations:
function _withdrawStrategyFunds(
uint256 amount_,
address receiver_,
address asset_,
Strategy[] memory strategies,
IParkingLot parkingLot
) internal {
// [...]
uint256 float = _asset.balanceOf(address(this));
if (amount_ <= float) {
bool result = TokenHelper.attemptSafeTransfer(address(asset_), receiver_, amount_, false);
// [...]
} else {
uint256 diff = amount_ - float;
uint256 totalWithdrawn = 0;
uint256 len = strategies.length;
for (uint256 i; i < len;) {
// [...]
uint256 amountToWithdraw = _calculateWithdrawalAmount(amount_, strategy);
// [...]
try strategy.strategy.withdraw(amountToWithdraw, receiver_, address(this)) {}
catch {
// [...]
}
totalWithdrawn += amountToWithdraw;
unchecked {
i++;
}
}
if (totalWithdrawn < amount_ && amount_ - totalWithdrawn <= float) {
uint256 net = amount_ - totalWithdrawn;
bool result = TokenHelper.attemptSafeTransfer(address(asset_), receiver_, net, false);
// [...]
}
}
}
function _calculateWithdrawalAmount(uint256 amount_, Strategy memory strategy) internal pure returns (uint256) {
return amount_.mulDiv(strategy.allocation.amount, MAX_BASIS_POINTS, Math.Rounding.Floor);
}
In the latter case, if withdrawals from the strategies are insufficient for the requested amount, the function withdraws the remaining amount from the vault's asset balance. However, if insufficient asset balance remains in the vault, the function proceeds without reverting. Consequently, users receive less than they are entitled to.
Impact
Users receive less than they are entitled to if the asset balance in the vault is insufficient for the remaining amount after proportional withdrawals from the strategies.
The following scenario illustrates this issue. Assume the Concrete Vault has 20% allocation to strategy X and 30% allocation to strategy Y.
Alice and Bob each deposit 500 tokens to the vault. The vault then contains 200 tokens in strategy X, 300 tokens in strategy Y, and 500 tokens in the vault balance.
Bob withdraws 500 tokens from the vault. The
_withdrawStrategyFunds
function transfers 500 tokens from the vault's balance to Bob. After the withdrawal, zero asset balance remains in the vault.Alice requests a withdrawal of 500 tokens. The
_withdrawStrategyFunds
function transfers 100 tokens (500 × 20%) from strategy X to Alice and transfers 150 tokens (500 × 30%) from strategy Y to Alice, then proceeds with execution.
As a result, Alice burns shares worth 500 tokens but only receives 250 tokens during withdrawal.
Recommendations
Consider reverting if the asset balance in the contract is insufficient for the remaining amount after proportional withdrawals from the strategies:
if (totalWithdrawn < amount_ && amount_ - totalWithdrawn <= float) {
uint256 net = amount_ - totalWithdrawn;
bool result = TokenHelper.attemptSafeTransfer(address(asset_), receiver_, net, false);
if (!result) {
parkingLot.deposit(receiver_, net);
}
- }
+ } else {revert("...");}
Remediation
This issue has been acknowledged by Blueprint Finance, and a fix was implemented in commit 4bc00f6a↗.
Blueprint Finance provided the following response to this finding:
The
_withdrawStrategyFunds
function has been restructured. The strategy allocation percentage is now calculated relative to the total strategy allocation, rather than usingMAX_BASIS_POINTS
.