H-03 MitigationConfirmed

[c4-bot-6]

Lines of code

Vulnerability details

The fix applied by the team fully mitigates H-03.

There is a risk of reentrancy introduced with the change, however with low severity because this scenario would only be possible to the admins who are trusted anyways.

[alcueca]

The mitigation review should include more than just links to the issue and the fix. Not much is needed, but at least a description of both.

[c4-judge]

alcueca marked the issue as unsatisfactory: Insufficient quality

[s1n1st3r01]

Original vulnerability

---

In the OperatorDelegator.sol contract, both completeQueuedWithdrawal() and receive() functions had the nonReentrant modifier. The purpose of having the modifier on both functions was to guard against reentrancy but this lead to the ETH withdrawal functionality being unusable entirely. Any withdrawals involving ETH will permanently fail because the nonReentrant modifier is triggered twice

Execution flow was as follows:

1. After the admin has requested a withdrawal and the withdrawal time window has passed, the admin via the nativeEthRestakeAdmin EOA address, calls the function completeQueuedWithdrawal() in OperatorDelegator.sol to finalize the withdrawal. Note that the reentrancy guard will be activated for the first time and will mark _status = _ENTERED.

      function completeQueuedWithdrawal(
       IDelegationManager.Withdrawal calldata withdrawal,
       IERC20[] calldata tokens,
       uint256 middlewareTimesIndex
   ) external nonReentrant onlyNativeEthRestakeAdmin {
       .............
       // Last argument referring to `receiveAsToken` which is set to `true
       delegationManager.completeQueuedWithdrawal(withdrawal, tokens, middlewareTimesIndex, true);
   

2. The DelegationManager.sol contract will end up calling the withdrawSharesAsTokens() function in the EigenPodManager.sol contract (in EigenLayer contracts)

3. The withdrawRestakedBeaconChainETH() function in EigenPod.sol will be called in the end, and this function will reach out to the OperatorDelegator.sol

4. That's when the receive() function in the OperatorDelegator.sol gets called and since it has the nonReentrant modifier, the reentrancy guard will be activated for the second time but it will revert, because the _status won't be _NOT_ENTERED anymore.

---

Mitigation Analysis

The mitigation presented here successfully mitigates the issue by removing the nonReentrant modifier from the receive() function, which allows it to be executed normally.

-  receive() external payable nonReentrant {
+  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

[c4-judge]

alcueca marked the issue as satisfactory