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

Bypassing Withdrawal Timing Restriction Due to Improper Update of timeOfLastWithdrawal

mahivasisth

Summary

The performUpkeep() function relies on timeOfLastWithdrawal to enforce a minimum time interval between consecutive withdrawal finalizations. However, the deposit() function in WithdrawalPool.sol also calls _finalizeWithdrawals() without updating timeOfLastWithdrawal. This allows users to bypass the timing restriction and finalize withdrawals immediately after making a small deposit, breaking the intended time-based security feature.

Vulnerability Details

In the performUpkeep() function, the following condition checks whether sufficient time has passed since the last withdrawal to prevent immediate repeated withdrawals:

https://github.com/Cyfrin/2024-09-stakelink/blob/f5824f9ad67058b24a2c08494e51ddd7efdbb90b/contracts/core/priorityPool/WithdrawalPool.sol#L348-L364

function performUpkeep(bytes calldata _performData) external {
uint256 canWithdraw = priorityPool.canWithdraw(address(this), 0);
uint256 totalQueued = _getStakeByShares(totalQueuedShareWithdrawals);
if (
totalQueued == 0 ||
canWithdraw == 0 ||
// @audit : user can _finalizeWithdrawals only after a certain minTimeBetweenWithdrawals
@-> 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);
}
​

https://github.com/Cyfrin/2024-09-stakelink/blob/f5824f9ad67058b24a2c08494e51ddd7efdbb90b/contracts/core/priorityPool/WithdrawalPool.sol#L323-L327

function deposit(uint256 _amount) external onlyPriorityPool {
token.safeTransferFrom(msg.sender, address(this), _amount);
lst.safeTransfer(msg.sender, _amount);
@-> _finalizeWithdrawals(_amount);
}

https://github.com/Cyfrin/2024-09-stakelink/blob/f5824f9ad67058b24a2c08494e51ddd7efdbb90b/contracts/core/priorityPool/WithdrawalPool.sol#L422-L466

// @audit-issue : timeOfLastWithdrawal is not updatd anywhere .
function _finalizeWithdrawals(uint256 _amount) internal {
uint256 sharesToWithdraw = _getSharesByStake(_amount);
uint256 numWithdrawals = queuedWithdrawals.length;
​
totalQueuedShareWithdrawals -= sharesToWithdraw;
​
for (uint256 i = indexOfNextWithdrawal; i < numWithdrawals; ++i) {
uint256 sharesRemaining = queuedWithdrawals[i].sharesRemaining;
​
if (sharesRemaining < sharesToWithdraw) {
// fully finalize withdrawal
sharesToWithdraw -= sharesRemaining;
continue;
}
​
if (sharesRemaining > sharesToWithdraw) {
// partially finalize withdrawal
queuedWithdrawals[i] = Withdrawal(
uint128(sharesRemaining - sharesToWithdraw),
uint128(
queuedWithdrawals[i].partiallyWithdrawableAmount +
_getStakeByShares(sharesToWithdraw)
)
);
indexOfNextWithdrawal = i;
withdrawalBatches.push(
WithdrawalBatch(uint128(i - 1), uint128(_getStakeByShares(1 ether)))
);
} else {
// fully finalize withdrawal
indexOfNextWithdrawal = i + 1;
withdrawalBatches.push(
WithdrawalBatch(uint128(i), uint128(_getStakeByShares(1 ether)))
);
}
​
sharesToWithdraw = 0;
break;
}
​
// entire amount must be accounted for
assert(sharesToWithdraw == 0);
​
emit WithdrawalsFinalized(_amount);
}
​

Impact

  • Bypassing timing restrictions: Users can bypass the intended time gap (minTimeBetweenWithdrawals) for finalizing withdrawals by making small deposits to the WithdrawalPool.

  • Potential abuse of the system: This flaw allows users to circumvent cooldown periods designed to prevent rapid consecutive withdrawals, potentially leading to withdrawal abuse or exploitation.

Tools Used

Manual review

Recommendations

Update timeOfLastWithdrawal in _finalizeWithdrawals() rather than in performUpkeep(). This way, whether _finalizeWithdrawals() is called via performUpkeep() or deposit(), the timeOfLastWithdrawal is always updated, ensuring that the minimum time between consecutive withdrawals is respected.

Updates

Lead Judging Commences

inallhonesty Lead Judge
about 1 year ago
inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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