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

updateWithdrawalBatchIdCutoff should be called before calling getBatchids

Dup1337

Summary

The irregularity occurs in the withdrawal batching mechanism of the contract where old WithdrawalBatchIdCutoff values can be incorrectly parsed or utilized in getBatchIds call.
Calling this function without updating the withdrawalBatchIdCutoff variable will lead to false batch id parsing.

Vulnerability Details

The contract manages withdrawals using a batching mechanism, where withdrawals are grouped into batches identified by batch IDs.
These batch IDs are crucial for tracking and processing withdrawals accurately.

However, the issue is in where old WithdrawalBatchIdCutoff values can be parsed or used if there has been a call to _finalizeWithdrawals without properly updating the batch IDs. This can result in outdated or incorrect batch IDs being parsed in getBatchIds call.

Contract: WithdrawalPool.sol
​
152: function getBatchIds(uint256[] memory _withdrawalIds) public view returns (uint256[] memory) {
153: uint256[] memory batchIds = new uint256[]();
154:
155: for (uint256 i = 0; i < _withdrawalIds.length; ++i) {
156: uint256 batchId;
157: uint256 withdrawalId = _withdrawalIds[i];
158:
159: >>> for (uint256 j = withdrawalBatchIdCutoff; j < withdrawalBatches.length; ++j) {
160: uint256 indexOfLastWithdrawal = withdrawalBatches[j].indexOfLastWithdrawal; //@audit-ok this could provide wrong id if the withdrawal id is provided wrongly
161:
162: if (withdrawalId <= indexOfLastWithdrawal) {// @audit-ok shouldnt it be only < ?
163: batchId = j;
164: break;
165: }
166: }
167:
168: batchIds[i] = batchId;
169: }
170:
171: return batchIds;
172: }

As can be seen on L:159, withdrawalBatchIdCutoff is used directly and assumed to be up to date which might not be the case since updateWithdrawalBatchIdCutoff is not triggered anywhere before.

The variable only updated in updateWithdrawalBatchIdCutoff:

Contract: WithdrawalPool.sol
​
370: function updateWithdrawalBatchIdCutoff() external {
371: uint256 numWithdrawals = queuedWithdrawals.length;
372: uint256 newWithdrawalIdCutoff = withdrawalIdCutoff;
373:
374: // find the first withdrawal that has funds remaining
375: for (uint256 i = newWithdrawalIdCutoff; i < numWithdrawals; ++i) {
376: newWithdrawalIdCutoff = i;
377:
378: Withdrawal memory withdrawal = queuedWithdrawals[i];
379: if (withdrawal.sharesRemaining != 0 || withdrawal.partiallyWithdrawableAmount != 0) {
380: break;
381: }
382: }
383:
384: uint256 numBatches = withdrawalBatches.length;
385: uint256 newWithdrawalBatchIdCutoff = withdrawalBatchIdCutoff;
386:
387: // find the last batch where all withdrawals have no funds remaining
388: for (uint256 i = newWithdrawalBatchIdCutoff; i < numBatches; ++i) {
389: if (withdrawalBatches[i].indexOfLastWithdrawal >= newWithdrawalIdCutoff) {
390: break;
391: }
392:
393: newWithdrawalBatchIdCutoff = i;
394: }
395:
396: withdrawalIdCutoff = uint128(newWithdrawalIdCutoff);
397: withdrawalBatchIdCutoff = uint128(newWithdrawalBatchIdCutoff);
398: }

and not explicitly called anywhere in the codebase.

Impact

Incorrect batch Id parsing in view functions

Tools Used

Manual Review

Recommendations

_finalizeWithdrawals could call updateWithdrawalBatchIdCutoff before syncing the state.

Updates

Lead Judging Commences

inallhonesty Lead Judge 7 months ago
Submission Judgement Published
Validated
Assigned finding tags:

newWithdrawalBatchIdCutoff

Support

FAQs

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