Liquid Staking

Description:

The function checkUpkeep is responsible for determining whether a withdrawal operation should be executed, based on the current state of the contract and the elapsed time since the last withdrawal. However, there is a flaw in the time comparison logic where the contract does not allow for upkeep if block.timestamp == timeOfLastWithdrawal + minTimeBetweenWithdrawals. This strict greater-than check (block.timestamp > timeOfLastWithdrawal + minTimeBetweenWithdrawals) introduces a one-block delay where upkeep is not permitted, which could be unintended.

This logic flaw might occur if the contract assumes withdrawals should be allowed exactly after the minimum time interval has passed but fails to account for this case in the code.

https://github.com/Cyfrin/2024-09-stakelink/blob/ea5574ebce3a86d10adc2e1a5f6d5512750f7a72/contracts/core/priorityPool/WithdrawalPool.sol#L329-L342

Impact:

This issue could lead to the following impact:

• Delay in withdrawals: Withdrawal operations will not be executed during the exact block when the condition block.timestamp == timeOfLastWithdrawal + minTimeBetweenWithdrawals is true. This could cause a delay of one block (or more if the condition is repeatedly missed) in processing withdrawals, potentially affecting user experience, liquidity availability, or automated contract operations.

While this issue does not lead to a severe security risk, it does degrade the expected behavior and performance of the contract.

Proof of Concept:

1. Assume:

   • timeOfLastWithdrawal = 1000

   • minTimeBetweenWithdrawals = 50

   • Current block.timestamp = 1050 (this is exactly at the threshold for the next allowed withdrawal)

2. The check:

   block.timestamp > timeOfLastWithdrawal + minTimeBetweenWithdrawals

   evaluates to:

   1050 > 1050 // false

   This prevents the withdrawal, even though it should be allowed at this exact timestamp.

3. The withdrawal will only be processed starting from block.timestamp = 1051, which introduces a one-block delay.

Recommended Mitigation:

To avoid this issue, modify the time comparison logic to allow withdrawals when the current timestamp is equal to or greater than the sum of timeOfLastWithdrawal and minTimeBetweenWithdrawals. Change the condition in the code as follows:

This ensures that the upkeep is checked as valid exactly when the minimum time interval has passed, avoiding any unnecessary delay in processing withdrawals.