Summary

In the _depositQueuedTokens (L-693) funtion in the PriorityPool, there are two arguments: _depositMin and _depositMax. These two arguments work together but are paradoxical. On one hand, the minDeposit causes the function to revert if the deposit room, or the total that can be deposited, is less than the minimum deposit. On the other hand, the depositMax checks the amount that will be transferred from the staking pool to the strategies and the substract this amount from the quantity allowed to be deposited from the queue as we can see in the _depositQueuedTokens function

The problem is that when a deposit is called in the staking pool, any unused deposit is still staked, as long as there is enough deposit room which mean that the deposit max is useless.

Vulnerabilty Details

imagine than the strategyDepositRoom = 10000 link token, _totalQueued = 2000 link token, unusedDeposits= 8000 linkToken and _depositMax =5000.

strategyDepositRoom>_depositMax &&unusedDeposits>_depositMax =>toDepositFromStakingPool ==_depositMax

=>toDepositFromQueue ==0

So nothing will be deposited from the queue and the amount deposited in the strategies will be eqal to 8000 link token.

8000>_depositMax this limit don't make any sense.

Impact

it will be difficult for the keeper to monitor the contract since the queue deposit limits are state variable used in the

performUpkeep(L-437) function and some shares that could be minted because there is a unused deposit limit in the staking pool it's better for the user to mint shares.

Tools Used

Echidna

Recommendations

It will be better to refactor the _depositQueuedTokens in order to take into account the fact that anyway the unused deposits will be deposited.