Liquid Staking

Stakelink
DeFiHardhatOracle
50,000 USDC
View results
Submission Details
Severity: high
Invalid

Underflow in `WithdrawalPool:_finalizeWithdrawals` function leading to DOS

Github

Summary

The _finalizeWithdrawals function in the WithdrawalPool contract contains a vulnerability that can cause a transaction to revert due to an underflow error when processing withdrawal batches. Specifically, the issue occurs when the function attempts to update the withdrawalBatches array with an index value derived from i - 1 during the first iteration when i is 0. This vulnerability can lead to a Denial-of-Service (DoS) condition as any attempts to process withdrawals when the contract is in this state will consistently revert, preventing users from fulfilling their withdrawal requests.

Vulnerability Details

The vulnerability is located in the _finalizeWithdrawals function of the WithdrawalPool contract, specifically at the following line:

withdrawalBatches.push(
WithdrawalBatch(uint128(i - 1), uint128(_getStakeByShares(1 ether)))
);

The issue arises during the first iteration of the loop when i is set to 0. The subtraction operation i - 1 causes an underflow in unsigned integer arithmetic, resulting in an attempt to convert -1 into a uint128, which is invalid. In Solidity 0.8.0 and later, this operation will cause the transaction to revert due to the built-in overflow/underflow checks.

If sharesToWithdraw (the amount of shares to be processed for withdrawal) is less than sharesRemaining (the amount of shares left in a specific withdrawal request), the code enters the block that processes partial withdrawals:

if (sharesRemaining > sharesToWithdraw) {
queuedWithdrawals[i] = Withdrawal(
uint128(sharesRemaining - sharesToWithdraw),
uint128(
queuedWithdrawals[i].partiallyWithdrawableAmount +
_getStakeByShares(sharesToWithdraw)
)
);
indexOfNextWithdrawal = i;
withdrawalBatches.push(
WithdrawalBatch(uint128(i - 1), uint128(_getStakeByShares(1 ether)))
);
}

When i = 0, the expression i - 1 underflows, resulting in a revert due to Solidity's overflow/underflow protection mechanisms. As a result, the function will consistently revert, preventing any further processing of withdrawal requests. The reversion will always occur during the first iteration of the loop if indexOfNextWithdrawal is set to 0 and the condition sharesRemaining > sharesToWithdraw is met.

Impact

The immediate consequence of the underflow is that the transaction will revert, preventing the function from processing any withdrawal requests (DOS). This issue will not only impact a single transaction but will consistently cause the contract to revert whenever this condition is met, effectively locking the withdrawal functionality. The inability to process withdrawals could lead to liquidity issues within the platform, as users cannot retrieve their staked or deposited tokens when they want to.

Tools Used

Manual Review

Recommendations

Modify the line that performs the subtraction to ensure that it does not cause an underflow:

uint256 batchIndex = i > 0 ? i - 1 : 0; // Prevent underflow by ensuring i is not zero
withdrawalBatches.push(
WithdrawalBatch(uint128(batchIndex), uint128(_getStakeByShares(1 ether)))
);
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.

Give us feedback!