Liquid Staking

Summary

PriorityPool::_depositQueuedTokens can potentially emit wrong deposit events and also break the intended behaviour.

Vulnerability Details

There can be a condition where PriorityPool::_depositQueuedTokens can potentially emit wrong event that can mislead the off-chain mechanism of the protocol into believing that there was a successful deposit.

Replicating the behaviour below:

A user / malicious actor calls the depositQueuedTokens.

Assumptions:

• _depositMin = 2 (minimum required deposit passed by the user)

• _depositMax = 1 (maximum allowed deposit passed by the user)

• _data = some bytes array (passed by the user, irrelevant for the logic)

• unusedDeposits = 3 (just an assumption for the dry run)

• strategyDepositRoom = 5 (the room left for deposits)

• totalQueued = 4 (tokens queued for deposit)

Dry Run Steps:

1. Initial Checks:

   • The pool status check passes (poolStatus == PoolStatus.OPEN).

   strategyDepositRoom = 5 and _depositMin = 2, so the condition:

   if (strategyDepositRoom == 0 || strategyDepositRoom < _depositMin) revert InsufficientDepositRoom();

   • does not revert because strategyDepositRoom(5) is greater than _depositMin(2).

2. CanDeposit Calculation:

   canDeposit = _totalQueued + unusedDeposits = 4 + 3 = 7

   Next, the condition:

   if (canDeposit == 0 || canDeposit < _depositMin) revert InsufficientQueuedTokens();

   checks if canDeposit(7) is less than _depositMin(2). Since it is not, the function proceeds without reverting.

3. Deposit Calculations:

   The function calculates toDepositFromStakingPool:

   toDepositFromStakingPool = MathUpgradeable.min(

   MathUpgradeable.min(unusedDeposits, strategyDepositRoom),

   _depositMax

   );

   MathUpgradeable.min(unusedDeposits(3), strategyDepositRoom(5)) = 3
MathUpgradeable.min(3, _depositMax(1)) = 1
   So, toDepositFromStakingPool = 1.

   Now, the function calculates toDepositFromQueue

   toDepositFromQueue = MathUpgradeable.min(

   MathUpgradeable.min(_totalQueued, strategyDepositRoom - toDepositFromStakingPool),

   _depositMax - toDepositFromStakingPool

   );

   • MathUpgradeable.min(_totalQueued(4), strategyDepositRoom(5) - toDepositFromStakingPool(1)) = MathUpgradeable.min(4, 4) = 4

   • But, _depositMax - toDepositFromStakingPool = 1 - 1 = 0, so toDepositFromQueue = MathUpgradeable.min(4, 0) = 0.

4. Deposit Execution:

   The function proceeds to execute the deposit:

   stakingPool.deposit(address(this), toDepositFromQueue, _data);

   This will call the stakingPool.deposit function with toDepositFromQueue = 0, so no tokens from the queue are actually deposited.

5. Update and Event Emission:

   _totalQueued -= toDepositFromQueue = 4 - 0 = 4.

   The condition _totalQueued != totalQueued does not trigger, so no updates to depositsSinceLastUpdate or sharesSinceLastUpdate occur.

   Finally, the event:

   emit DepositTokens(toDepositFromStakingPool, toDepositFromQueue);

   will be emitted as:

   emit DepositTokens(1, 0);

Conclusion:

This behaviour is a clear example of a misleading event, this could lead to misleading assumptions by the offchain mechanism.

Impact

Corrupts the protocol's off-chain data.

Tools Used

Manual review

Recommendations

Modify the logic of the depositQueuedTokens.