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

0 stars 0 forks source link

H-03 MitigationConfirmed #19

Open c4-bot-9 opened 1 month ago

c4-bot-9 commented 1 month ago

Lines of code

Vulnerability details

See:

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

Navigating to H-03 from the previous contest we can see that there was an issue in the OperatorDelegator.completeQueuedWithdrawal() function in the OperatorDelegator.sol contract which is used by admins to finalize withdrawals of shares from EigenLayer.

The crux of the issue was the fact that both the completeQueuedWithdrawal() & receive() function of the OperatorDelegator were marked as nonReentrant, which prevents reentrancy but this also blocks necessary reentrant calls for ETH redemptions from EigenLayer. As a result, any withdrawals involving ETH will permanently fail as the nonReentrant modifier is executed twice while still entered, potentially even leading to insolvency, necessitating an upgrade to OperatorDelegator to resolve this issue.

Now, protocol have sufficiently fixed the issue via this pull request where it removed the nonReentrant modifier from the OperatorDelegator#receive() function, which now means that a revert would not occur, i.e:

     * @dev Handle ETH sent to this contract - will get forwarded to the deposit queue for restaking as a protocol reward
      * @dev If msg.sender is eigenPod then forward ETH to deposit queue without taking cut (i.e. full withdrawal from beacon chain)
      */
-   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 }();
c4-judge commented 4 weeks ago

alcueca marked the issue as satisfactory