Summary
Allowing accounts to withdraw full balance in function StakingPool#withdraw() is redundant because caller does not have enough balance
Vulnerability Details
The function StakingPool#withdraw() allows the caller to withdraw all balance if setting _amount = type(uint256).max. The function can be triggered when transaction originated from WithdrawalPool#performUpkeep(). The function WithdrawalPool#performUpkeep() calls PriorityPool#executeQueuedWithdrawals(), which will pull _amount from WithdrawalPool and then call StakingPool#withdraw() with the mentioned amount.
The logic in function StakingPool#withdraw() below
if (_amount == type(uint256).max) {
toWithdraw = balanceOf(_account);
}
will be redundant. Because if _amount is set to type(uint256).max then the transaction always reverts because not enough balance.
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 executeQueuedWithdrawals(
uint256 _amount,
bytes[] calldata _data
) external onlyWithdrawalPool {
@> IERC20Upgradeable(address(stakingPool)).safeTransferFrom(
msg.sender,
address(this),
_amount
);
@> stakingPool.withdraw(address(this), address(this), _amount, _data);
token.safeTransfer(msg.sender, _amount);
}
function withdraw(
address _account,
address _receiver,
uint256 _amount,
bytes[] calldata _data
) external onlyPriorityPool {
uint256 toWithdraw = _amount;
@> if (_amount == type(uint256).max) {
toWithdraw = balanceOf(_account);
}
uint256 balance = token.balanceOf(address(this));
if (toWithdraw > balance) {
_withdrawLiquidity(toWithdraw - balance, _data);
}
require(
token.balanceOf(address(this)) >= toWithdraw,
"Not enough liquidity available to withdraw"
);
_burn(_account, toWithdraw);
totalStaked -= toWithdraw;
token.safeTransfer(_receiver, toWithdraw);
}
Impact
Redundant logic
Tools Used
Manual
Recommendations
Consider removing the logic
