code-423n4 / 2024-06-renzo-mitigation-findings

0 stars 0 forks source link

H-03 MitigationConfirmed #10

Open c4-bot-2 opened 1 month ago

c4-bot-2 commented 1 month ago

Lines of code

Vulnerability details

C4 issue

H-03: ETH withdrawals from EigenLayer always fail due to OperatorDelegator's nonReentrant receive()

Link to issue

Comments

The completeQueuedWithdrawal() function in the OperatorDelegator contract is used to finalize any queued withdrawals. This function has a nonReentrant modifier, which activates the reentrant lock once called. The issue is since receiveAsTokens == true, when ETH is being withdrawn, the receive() function in the same contract is called by the EigenPod to receive the funds. However, since the receive() function also implements the nonReentrant modifier, the function fails due to the reentrancy lock.

Mitigation

PR: Pull Request 87 - H03FIX

The fix removes the nonReentrant modifier from the receive() function. This allows the function to execute successfully without being blocked by the reentrancy lock.

    receive() external payable {
        // check if sender contract is EigenPod. forward full withdrawal eth received
        if (msg.sender == address(eigenPod)) {
            restakeManager.depositQueue().forwardFullWithdrawalETH{ value: msg.value }();
        } else {
            // considered as protocol reward
            uint256 gasRefunded = 0;
            uint256 remainingAmount = msg.value;
            if (adminGasSpentInWei[tx.origin] > 0) {
                gasRefunded = _refundGas();
                // update the remaining amount
                remainingAmount -= gasRefunded;
                // If no funds left, return
                if (remainingAmount == 0) {
                    return;
                }
            }
            // Forward remaining balance to the deposit queue
            address destination = address(restakeManager.depositQueue());
            (bool success, ) = destination.call{ value: remainingAmount }("");
            if (!success) revert TransferFailed();

            emit RewardsForwarded(destination, remainingAmount);
        }
    }

Test

New test cases have been added to verify that the receive() function correctly processes ETH withdrawals without the nonReentrant modifier. All tests have passed, confirming the fix.

Contract: OperatorDelegatorForkTest

Tests:

Conclusion

Removing the nonReentrant modifier from the receive() function resolves the issue of failed ETH withdrawals from EigenLayer.

c4-judge commented 4 weeks ago

alcueca marked the issue as satisfactory

c4-judge commented 4 weeks ago

alcueca marked the issue as confirmed for report