Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Extending/reducing cooldown period affects already queued withdrawals #609

Closed
howlbot-integration bot opened this issue May 28, 2024 · 4 comments
Closed
Labels
bug Something isn't working downgraded by judge Judge downgraded the risk level of this issue duplicate-607 grade-b QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax 🤖_31_group AI based duplicate group recommendation sufficient quality report This report is of sufficient quality

Comments

@howlbot-integration
Copy link

Lines of code

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

Vulnerability details

Descritpion

Whene users initiated a withdrawl they are expecting to wait until current cooldown period ends to be able to claim their funds. However if the current cool down period is modified, this will affect this time.

Impact

Arbitrary extension of cooldown period can affect users that are waiting for their funds to be available.

POC

Call of WitdrawQueue.updateCoolDownPeriod(uint256 _newCoolDownPeriod) will affect current queued withdrawls extending their duration of coolDownPeriod < _newCoolDownPeriod given current implementation of function claim

Recommended mitigation

Record the current cooldown period when a user initiate a withdrawl and use this value to calculate the time when the funds will be available to claim.

    function claim(uint256 withdrawRequestIndex) external nonReentrant {
        // ...
-       if (block.timestamp - _withdrawRequest.createdAt < coolDownPeriod) revert EarlyClaim();
+        if (block.timestamp - _withdrawRequest.createdAt < _withdrawRequest.coolDownPeriod) revert EarlyClaim();
        //...
    }

Assessed type

Other

@howlbot-integration howlbot-integration bot added 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value 🤖_31_group AI based duplicate group recommendation bug Something isn't working sufficient quality report This report is of sufficient quality labels May 28, 2024
howlbot-integration bot added a commit that referenced this issue May 28, 2024
@c4-judge
Copy link
Contributor

alcueca marked the issue as duplicate of #607

@c4-judge
Copy link
Contributor

alcueca marked the issue as unsatisfactory:
Invalid

@c4-judge c4-judge added unsatisfactory does not satisfy C4 submission criteria; not eligible for awards downgraded by judge Judge downgraded the risk level of this issue QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax and removed 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value labels May 30, 2024
@c4-judge
Copy link
Contributor

alcueca changed the severity to QA (Quality Assurance)

@c4-judge
Copy link
Contributor

alcueca marked the issue as grade-b

@c4-judge c4-judge added grade-b and removed unsatisfactory does not satisfy C4 submission criteria; not eligible for awards labels May 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working downgraded by judge Judge downgraded the risk level of this issue duplicate-607 grade-b QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax 🤖_31_group AI based duplicate group recommendation sufficient quality report This report is of sufficient quality
Projects
None yet
Development

No branches or pull requests

1 participant