Description
WithdrawalPool::withdraw allows the execution of a group of fully and/or partially finalized withdrawals owned by the sender. However, this function never calls WithdrawalPool::_finalizeWithdrawals, failing to update totalQueuedShareWithdrawals.
For every withdrawal with that function, totalQueuedShareWithdrawals won't be updated and will increase indefinitely, but in reality, tokens will be transferred.
function withdraw(uint256[] calldata _withdrawalIds, uint256[] calldata _batchIds) external {
address owner = msg.sender;
uint256 amountToWithdraw;
for (uint256 i = 0; i < _withdrawalIds.length; ++i) {
uint256 withdrawalId = _withdrawalIds[i];
Withdrawal memory withdrawal = queuedWithdrawals[_withdrawalIds[i]];
uint256 batchId = _batchIds[i];
WithdrawalBatch memory batch = withdrawalBatches[batchId];
if (withdrawalOwners[withdrawalId] != owner) revert SenderNotAuthorized();
if (
batchId != 0 && withdrawalId <= withdrawalBatches[batchId - 1].indexOfLastWithdrawal
) revert InvalidWithdrawalId();
if (
batchId != 0 &&
withdrawalId > batch.indexOfLastWithdrawal &&
withdrawal.partiallyWithdrawableAmount == 0
) revert InvalidWithdrawalId();
if (withdrawalId <= batch.indexOfLastWithdrawal) {
amountToWithdraw +=
withdrawal.partiallyWithdrawableAmount +
(uint256(batch.stakePerShares) * uint256(withdrawal.sharesRemaining)) /
1e18;
delete queuedWithdrawals[withdrawalId];
delete withdrawalOwners[withdrawalId];
} else {
amountToWithdraw += withdrawal.partiallyWithdrawableAmount;
queuedWithdrawals[withdrawalId].partiallyWithdrawableAmount = 0;
}
}
token.safeTransfer(owner, amountToWithdraw);
emit Withdraw(owner, amountToWithdraw);
}
This will have two real impact:
performUpkeep will always set toWithdraw to canWithdraw, even when the the contract balance is not enough to proceed.
getTotalQueuedWithdrawals will always be positive and, any deposit in the PriorityPool will lead to sending the token to WithdrawalPool, even if there is no queued withdrawal, instead of staking them in the Chainlink contract.
function performUpkeep(bytes calldata _performData) external {
uint256 canWithdraw = priorityPool.canWithdraw(address(this), 0);
uint256 totalQueued = _getStakeByShares(totalQueuedShareWithdrawals);
if (
totalQueued == 0 ||
canWithdraw == 0 ||
block.timestamp <= timeOfLastWithdrawal + minTimeBetweenWithdrawals
) revert NoUpkeepNeeded();
timeOfLastWithdrawal = uint64(block.timestamp);
uint256 toWithdraw = totalQueued > canWithdraw ? canWithdraw : totalQueued;
bytes[] memory data = abi.decode(_performData, (bytes[]));
priorityPool.executeQueuedWithdrawals(toWithdraw, data);
_finalizeWithdrawals(toWithdraw);
}
function _deposit(
address _account,
uint256 _amount,
bool _shouldQueue,
bytes[] memory _data
) internal {
if (poolStatus != PoolStatus.OPEN) revert DepositsDisabled();
uint256 toDeposit = _amount;
if (totalQueued == 0) {
@> uint256 queuedWithdrawals = withdrawalPool.getTotalQueuedWithdrawals();
@> if (queuedWithdrawals != 0) {
@> uint256 toDepositIntoQueue = toDeposit <= queuedWithdrawals
@> ? toDeposit
@> : queuedWithdrawals;
@> withdrawalPool.deposit(toDepositIntoQueue);
@> toDeposit -= toDepositIntoQueue;
@> IERC20Upgradeable(address(stakingPool)).safeTransfer(_account, toDepositIntoQueue);
}
if (toDeposit != 0) {
uint256 canDeposit = stakingPool.canDeposit();
if (canDeposit != 0) {
uint256 toDepositIntoPool = toDeposit <= canDeposit ? toDeposit : canDeposit;
stakingPool.deposit(_account, toDepositIntoPool, _data);
toDeposit -= toDepositIntoPool;
}
}
)
...
}
...
}
Risk
Likelyhood: High.
Impact: High
Recommended Mitigation
Update totalQueuedShareWithdrawals in withdraw or use _finalizeWithdrawals.
