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

DoS Vulnerability in `WithdrawalPool:withdraw` Due to Lack of Validation in `setMinWithdrawalAmount`

farismaulana

Summary

The _minWithdrawalAmount parameter in WithdrawalPool::setMinWithdrawalAmount is not validated, allowing it to be set to a very small amount or zero. This can lead to abuse by cluttering the queuedWithdrawals and queuedWithdrawalsByAccount arrays, potentially causing a Denial of Service (DoS) when _finalizeWithdrawals is called.

Vulnerability Details

WithdrawalPool.sol#L107

function initialize(
address _token,
address _lst,
address _priorityPool,
uint256 _minWithdrawalAmount,
uint64 _minTimeBetweenWithdrawals
) public initializer {
.
.
.
​
@> minWithdrawalAmount = _minWithdrawalAmount;
}

WithdrawalPool.sol#L404-L407

function setMinWithdrawalAmount(uint256 _minWithdrawalAmount) external onlyOwner {
@> minWithdrawalAmount = _minWithdrawalAmount;
emit SetMinWithdrawalAmount(_minWithdrawalAmount);
}

there are missing _minWithdrawalAmount validation and if the value are too small (like 10 instead of 10e18) would lead to abuse/griefing the system by cluttering the queuedWithdrawals and queuedWithdrawalsByAccount array, potentially leading to DOS when performing huge amount of withdrawal array when performing _finalizeWithdrawals function.

the setup:

PriorityPool.sol#L311-L317

function _withdraw(address _account, uint256 _amount, bool _shouldQueueWithdrawal) internal returns (uint256) {
if (poolStatus == PoolStatus.CLOSED) revert WithdrawalsDisabled();
​
uint256 toWithdraw = _amount;
​
if (totalQueued != 0) {
uint256 toWithdrawFromQueue = toWithdraw <= totalQueued ? toWithdraw : totalQueued;
totalQueued -= toWithdrawFromQueue;
depositsSinceLastUpdate += toWithdrawFromQueue;
sharesSinceLastUpdate += stakingPool.getSharesByStake(toWithdrawFromQueue);
toWithdraw -= toWithdrawFromQueue;
}
​
@> if (toWithdraw != 0) {
if (!_shouldQueueWithdrawal) revert InsufficientLiquidity();
@> withdrawalPool.queueWithdrawal(_account, toWithdraw);
}
​
emit Withdraw(_account, _amount - toWithdraw);
return toWithdraw;
}

the requirement for the attack is to let toWithdraw > 0 when calling withdraw function and later _withdraw in PriorityPool. use 10 wei for example.
after the function would call withdrawalPool.queueWithdrawal with toWithdraw equal to 10. and this can be done multiple times with relative cheap.

WithdrawalPool.sol#L308

function queueWithdrawal(address _account, uint256 _amount) external onlyPriorityPool {
if (_amount < minWithdrawalAmount) revert AmountTooSmall();
​
lst.safeTransferFrom(msg.sender, address(this), _amount);
​
uint256 sharesAmount = _getSharesByStake(_amount);
@> queuedWithdrawals.push(Withdrawal(uint128(sharesAmount), 0)); // this would cluttered with small amount of withdrawal
totalQueuedShareWithdrawals += sharesAmount;
​
uint256 withdrawalId = queuedWithdrawals.length - 1;
queuedWithdrawalsByAccount[_account].push(withdrawalId); // also this
withdrawalOwners[withdrawalId] = _account;
​
emit QueueWithdrawal(_account, _amount);
}

when user call function that leads to _finalizeWithdrawals, it would iterate to the queuedWithdrawals and with the huge amount of the array there would be potentially out of gas scenario leading to the DoS

Impact

DoS when _finalizeWithdrawals is called by function deposit and performUpkeep, rendering the contract unusable

Tools Used

manual review

Recommendations

add the validation for _minWithdrawalAmount with minimum of reasonable amount of token so it would be costly to perform such attack.

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.