Liquid Staking
Stakelink
DeFiHardhatOracle
50,000 USDC
View results
Previous Next
Submission Details
Severity: medium
Invalid

`PriorityPool::setWithdrawalPool()` lacks necessary checks, potentially causing `share` to become stuck and preventing successful withdrawals.

joicygiore

Summary

PriorityPool::setWithdrawalPool() lacks necessary checks, potentially causing share to become stuck and preventing successful withdrawals.withdrawals.

Vulnerability Details

The setWithdrawalPool() function is designed to update the withdrawalPool address. However, it lacks necessary checks to ensure that the shares in the existing withdrawalPool contract queue have been fully processed. If users still have pending shares in the previous withdrawalPool, they may be unable to receive the corresponding stake tokens during withdrawal attempts.

// PriorityPool::setWithdrawalPool()
function setWithdrawalPool(address _withdrawalPool) external onlyOwner {
if (address(withdrawalPool) != address(0)) {
IERC20Upgradeable(address(stakingPool)).safeApprove(address(withdrawalPool), 0);
token.safeApprove(address(withdrawalPool), 0);
}
​
​
IERC20Upgradeable(address(stakingPool)).safeApprove(_withdrawalPool, type(uint256).max);
token.safeApprove(_withdrawalPool, type(uint256).max);
​
​
withdrawalPool = IWithdrawalPool(_withdrawalPool);
}
// PriorityPool::executeQueuedWithdrawals()
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);
}

The WithdrawalPool::performUpkeep() function is responsible for processing withdrawals. As indicated by the @> mark, it calls the PriorityPool::executeQueuedWithdrawals() function to retrieve the tokens. However, since this function is protected by the onlyWithdrawalPool modifier, only the currently assigned withdrawalPool can call it. If the withdrawalPool address is updated without ensuring all pending withdrawals have been processed, the previous withdrawalPool can no longer invoke this method, leaving the associated shares unwithdrawable.

// WithdrawalPool::performUpkeep()
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);
}

Code Snippet

https://github.com/Cyfrin/2024-09-stakelink/blob/f5824f9ad67058b24a2c08494e51ddd7efdbb90b/contracts/core/priorityPool/PriorityPool.sol#L579-L589

Impact

Updating the withdrawalPool without proper checks can result in users' shares being stuck in the old pool, making it impossible for them to complete withdrawals and retrieve their stake tokens. This could lead to user dissatisfaction and potential loss of funds.

Tools Used

Manual Review

Recommendations

To mitigate this issue, the following checks should be added to ensure that no pending withdrawals remain in the current withdrawalPool before updating it:

function setWithdrawalPool(address _withdrawalPool) external onlyOwner {
if (address(withdrawalPool) != address(0)) {
+ require(withdrawalPool.getTotalQueuedWithdrawals() != 0, "Total Queued Withdrawals Amount is not 0");
IERC20Upgradeable(address(stakingPool)).safeApprove(address(withdrawalPool), 0);
token.safeApprove(address(withdrawalPool), 0);
}
​
​
IERC20Upgradeable(address(stakingPool)).safeApprove(_withdrawalPool, type(uint256).max);
token.safeApprove(_withdrawalPool, type(uint256).max);
​
​
withdrawalPool = IWithdrawalPool(_withdrawalPool);
}
Updates

Lead Judging Commences

inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.

Give us feedback!