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

A negative LST rebase may cause getAvailableToWithdraw to revert blocking deposits and withdrawals #285

Closed
howlbot-integration bot opened this issue May 9, 2024 · 7 comments
Labels
3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working duplicate-282 🤖_39_group AI based duplicate group recommendation satisfactory satisfies C4 submission criteria; eligible for awards sufficient quality report This report is of sufficient quality upgraded by judge Original issue severity upgraded from QA/Gas by judge

Comments

@howlbot-integration
Copy link

Lines of code

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

Vulnerability details

Cause

WithdrawQueue's getAvailableToWithdraw function may underflow and revert if stETH (or another rebasing LST) experiences a negative rebase. This is because when IERC20(_asset).balanceOf(address(this)) - claimReserve[_asset] is 0 (or nearly 0), a negative rebase will result in stETH balance being lower than claimReserve.

A negative rebase is expected to happen due to slashing, inactivity leaks, or a Lido issue in the case of stETH. Other rebasing LSTs may also expirience a negative rebase during the course of normal operation.

Impact

This will cause the following user flows to revert:

This will also cause the following admin flow to revert:

Crucially, because user deposits will revert and OperatorDelegator.completeQueuedWithdrawal will revert, it will not be possible to refill the buffer. This is because refilling the buffer always calls the method to calculate the transfer amount, which will revert for this asset.

For the issue to resolve, unclaimed withdrawals will need to be claimed. However, due to the 7-day delay imposed on unclaiming, this may take up to 7 days. Even then, because a claim transaction may only be submitted by the original withdrawer (msg.sender check), a user may prolong the DoS maliciously or inadvertently by not claiming their withdrawal.

Even if the buffer is not near 0 (when balance and reserve are equal to each other), an attacker may frontrun a negative rebase with a deposit and a withdrawal to "take the buffer" bringing it to 0, such that after their action, the protocol will stop functioning for this asset, possibly until the attacker claims their withdrawal. This will result in a prolonged DoS.

Proof of Concept

  1. The buffer for stETH is at (or near) 0 following a withdrawal request "taking" it by increasing the claimReserve, either in the course of normal operations or maliciously.
  2. A negative rebase reduces the stETH supply held by the WithdrawQueue.
  3. All subsequent calls to the buffer views for stETH will revert, preventing refilling it.
  4. User deposits, withdrawals, and admin's claiming of EigenLayer withdrawal (OperatorDelegator.completeQueuedWithdrawal) involving stETH all revert until a sufficient amount of withdrawals is claimed to reduce the claimReserve.

Tools Used

Manual Review

Recommended Mitigation Steps

Check if the subtraction will underflow and report 0 in that case:

    function getAvailableToWithdraw(address _asset) public view returns (uint256) {
        if (_asset != IS_NATIVE) {
-            return IERC20(_asset).balanceOf(address(this)) - claimReserve[_asset];
+            uint256 balance = IERC20(_asset).balanceOf(address(this));
+            return (balance > claimReserve[_asset]) ? balance - claimReserve[_asset] : 0;

Assessed type

DoS

@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 🤖_39_group AI based duplicate group recommendation bug Something isn't working sufficient quality report This report is of sufficient quality labels May 9, 2024
howlbot-integration bot added a commit that referenced this issue May 9, 2024
@c4-judge c4-judge added 3 (High Risk) Assets can be stolen/lost/compromised directly upgraded by judge Original issue severity upgraded from QA/Gas by judge and removed 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value labels May 17, 2024
@c4-judge
Copy link
Contributor

alcueca changed the severity to 3 (High Risk)

@c4-judge
Copy link
Contributor

alcueca marked the issue as duplicate of #326

@c4-judge
Copy link
Contributor

alcueca marked the issue as satisfactory

@c4-judge c4-judge added the satisfactory satisfies C4 submission criteria; eligible for awards label May 17, 2024
@c4-judge c4-judge reopened this May 27, 2024
@c4-judge
Copy link
Contributor

alcueca marked the issue as not a duplicate

@c4-judge c4-judge added 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value downgraded by judge Judge downgraded the risk level of this issue and removed 3 (High Risk) Assets can be stolen/lost/compromised directly upgraded by judge Original issue severity upgraded from QA/Gas by judge labels May 27, 2024
@c4-judge
Copy link
Contributor

alcueca changed the severity to 2 (Med Risk)

@c4-judge
Copy link
Contributor

alcueca marked the issue as duplicate of #282

@c4-judge c4-judge added duplicate-282 3 (High Risk) Assets can be stolen/lost/compromised directly and removed 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value downgraded by judge Judge downgraded the risk level of this issue labels May 27, 2024
@c4-judge c4-judge added the upgraded by judge Original issue severity upgraded from QA/Gas by judge label May 27, 2024
@c4-judge
Copy link
Contributor

alcueca changed the severity to 3 (High Risk)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working duplicate-282 🤖_39_group AI based duplicate group recommendation satisfactory satisfies C4 submission criteria; eligible for awards sufficient quality report This report is of sufficient quality upgraded by judge Original issue severity upgraded from QA/Gas by judge
Projects
None yet
Development

No branches or pull requests

2 participants