Summary
In PriorityPool _depositQueuedTokens is used to deposit queued and/or unused tokens.
It accepts three parameters:
_depositMin - minimum amount of tokens required to deposit
_depositMax - max amount of tokens that can be deposited into at once
_data - deposit data passed to staking pool strategies
By the given definition we would expect that a call with _depositMin equal to 0 will go through even if the amount that can be deposited is 0. However, that's not the case. There is a special check to prevent this case and this will cause wrongly reverted transactions.
Vulnerability Details
function _depositQueuedTokens(
uint256 _depositMin,
uint256 _depositMax,
bytes[] memory _data
) internal {
if (poolStatus != PoolStatus.OPEN) revert DepositsDisabled();
uint256 strategyDepositRoom = stakingPool.getStrategyDepositRoom();
if (strategyDepositRoom == 0 || strategyDepositRoom < _depositMin)
revert InsufficientDepositRoom();
uint256 _totalQueued = totalQueued;
uint256 unusedDeposits = stakingPool.getUnusedDeposits();
uint256 canDeposit = _totalQueued + unusedDeposits;
if (canDeposit == 0 || canDeposit < _depositMin) revert InsufficientQueuedTokens();
uint256 toDepositFromStakingPool = MathUpgradeable.min(
MathUpgradeable.min(unusedDeposits, strategyDepositRoom),
_depositMax
);
uint256 toDepositFromQueue = MathUpgradeable.min(
MathUpgradeable.min(_totalQueued, strategyDepositRoom - toDepositFromStakingPool),
_depositMax - toDepositFromStakingPool
);
stakingPool.deposit(address(this), toDepositFromQueue, _data);
_totalQueued -= toDepositFromQueue;
if (_totalQueued != totalQueued) {
uint256 diff = totalQueued - _totalQueued;
depositsSinceLastUpdate += diff;
sharesSinceLastUpdate += stakingPool.getSharesByStake(diff);
totalQueued = _totalQueued;
}
emit DepositTokens(toDepositFromStakingPool, toDepositFromQueue);
}
If we look at this piece of code:
if (canDeposit == 0 || canDeposit < _depositMin) revert InsufficientQueuedTokens();
We will see that if _depositMin is equal to 0 and canDeposit is equal to 0 the call will revert even though the call was meant to pass even when canDeposit is equal to 0.
Impact
The impact of such an issue is wrongly reverted transactions.
Tools Used
Manual Review
Recommendations
Remove the logic reverting such a valid call and just skip the following logic when canDeposit is equal to 0:
function _depositQueuedTokens(
uint256 _depositMin,
uint256 _depositMax,
bytes[] memory _data
) internal {
if (poolStatus != PoolStatus.OPEN) revert DepositsDisabled();
uint256 strategyDepositRoom = stakingPool.getStrategyDepositRoom();
if (strategyDepositRoom == 0 || strategyDepositRoom < _depositMin)
revert InsufficientDepositRoom();
uint256 _totalQueued = totalQueued;
uint256 unusedDeposits = stakingPool.getUnusedDeposits();
uint256 canDeposit = _totalQueued + unusedDeposits;
- if (canDeposit == 0 || canDeposit < _depositMin) revert InsufficientQueuedTokens();
+ if (canDeposit < _depositMin) revert InsufficientQueuedTokens();
+ if (canDeposit != 0) {
uint256 toDepositFromStakingPool = MathUpgradeable.min(
MathUpgradeable.min(unusedDeposits, strategyDepositRoom),
_depositMax
);
uint256 toDepositFromQueue = MathUpgradeable.min(
MathUpgradeable.min(_totalQueued, strategyDepositRoom - toDepositFromStakingPool),
_depositMax - toDepositFromStakingPool
);
stakingPool.deposit(address(this), toDepositFromQueue, _data);
_totalQueued -= toDepositFromQueue;
if (_totalQueued != totalQueued) {
uint256 diff = totalQueued - _totalQueued;
depositsSinceLastUpdate += diff;
sharesSinceLastUpdate += stakingPool.getSharesByStake(diff);
totalQueued = _totalQueued;
}
emit DepositTokens(toDepositFromStakingPool, toDepositFromQueue);
+ }
}
