code-423n4 / 2024-04-renzo-findings

12 stars 8 forks source link

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

Closed howlbot-integration[bot] closed 5 months ago

howlbot-integration[bot] commented 6 months ago

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

c4-judge commented 6 months ago

alcueca changed the severity to 3 (High Risk)

c4-judge commented 6 months ago

alcueca marked the issue as duplicate of #326

c4-judge commented 6 months ago

alcueca marked the issue as satisfactory

c4-judge commented 5 months ago

alcueca marked the issue as not a duplicate

c4-judge commented 5 months ago

alcueca changed the severity to 2 (Med Risk)

c4-judge commented 5 months ago

alcueca marked the issue as duplicate of #282

c4-judge commented 5 months ago

alcueca changed the severity to 3 (High Risk)