Unfair changes to cooldown period affect existing withdrawals

[howlbot-integration]

Lines of code

https://github.com/code-423n4/2024-04-renzo/blob/main/contracts/Withdraw/WithdrawQueue.sol#L287

Vulnerability details

To prevent users from immediately claiming their withdrawals, the WithdrawQueue enforces a cooldown period during which the funds cannot be claimed. This cooldown period is stored in the coolDownPeriod variable and can be modified by the admin via the updateCoolDownPeriod() function.

The issue is that when a user goes to claim their withdrawal by calling claim(), the current value of coolDownPeriod is used to check if enough time has elapsed since the withdrawal was initiated:

if (block.timestamp - _withdrawRequest.createdAt < coolDownPeriod) revert EarlyClaim();

This means that if the cooldown period is increased after a user initiates a withdrawal, they will have to wait longer than expected to claim their funds. This is unfair as the terms of the withdrawal should be honored as they were when the user initiated it. Conversely, if the cooldown period is decreased, users can claim their withdrawals sooner than was originally enforced when they initiated the withdrawal.

Impact

Changes to the cooldown period will unexpectedly affect existing user withdrawals.

Proof of Concept

1. User initiates a withdrawal via withdraw() when the coolDownPeriod is set to 7 days
2. Admin calls updateCoolDownPeriod() to change the coolDownPeriod to 30 days
3. 7 days later, when the user tries to claim() their withdrawal, the transaction reverts with EarlyClaim
4. User has to wait a total of 30 days to claim their withdrawal, even though the original terms were 7 days

Tools Used

Manual review

Recommended Mitigation Steps

Instead of storing the timestamp at which the request was created as WithdrawRequest.createdAt, store the timestamp at which it can be claimed by adding the current cooldown period to block.timestamp on creation and storing it as e.g. unlockTime.

Assessed type

Other

[c4-judge]

alcueca marked the issue as primary issue

[alcueca]

@jatinj615, please review

[jatinj615]

Expected behaviour. as AVS slashing can enforce a longer cooldown. and protocol want the ability to price the existing withdrawals for slashing at time of claim.

[alcueca]

Usually, giving the protocol the option to change terms and conditions without giving users an option to bail out is considered a Medium. To avoid excessive reporting of issues that would invariably be found as expected behaviour by sponsors, I'm going to recommend the sponsor to clearly include in the terms and conditions that that cooldown period can be changed by governance, affecting withdrawals in the queue.

[c4-judge]

alcueca marked the issue as unsatisfactory:
Invalid

[c4-judge]

alcueca changed the severity to QA (Quality Assurance)

[c4-judge]

alcueca marked the issue as grade-b