M-09 MitigationConfirmed

[c4-bot-8]

Lines of code

Vulnerability details

The fix applied by the team fully mitigates M-09.

[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

---

The deposit() function in the RestakeManager.sol contract allows a user to deposit ERC20 collateral into the protocol and get ezETH in return. The issue is that function will revert if the withdraw buffer for the to-be-deposited ERC20 collateral token is greater than or equal to the amount of tokens deposited.

    function deposit(
        IERC20 _collateralToken,
        uint256 _amount,
        uint256 _referralId
    ) public nonReentrant notPaused {
        ................

        // Check the withdraw buffer and fill if below buffer target
        uint256 bufferToFill = depositQueue.withdrawQueue().getBufferDeficit(
            address(_collateralToken)
        );
        if (bufferToFill > 0) {
            bufferToFill = (_amount <= bufferToFill) ? _amount : bufferToFill;
            // update amount to send to the operator Delegator
------->    _amount -= bufferToFill;

            // safe Approve for depositQueue
            _collateralToken.safeApprove(address(depositQueue), bufferToFill);

            // fill Withdraw Buffer via depositQueue
            depositQueue.fillERC20withdrawBuffer(address(_collateralToken), bufferToFill);
        }

        // Approve the tokens to the operator delegator
        _collateralToken.safeApprove(address(operatorDelegator), _amount);

        // Call deposit on the operator delegator
----->  operatorDelegator.deposit(_collateralToken, _amount);

        .........
    }

1. The deposit() function firstly checks for the amount of withdraw buffer that needs to be filled
2. If the to-be-filled buffer amount is greater than zero, and it's greater than or equal to the amount of tokens deposited, then bufferToFill will be equal to the _amount variable. Then the bufferToFill will be subtracted from the _amount variable, making the _amount equal to zero now.

3. The deposit() function will then attempt to call the deposit() function in the OperatorDelegator contract, and supply the argument _amount as the amount of ERC20 collateral to be deposited in EigenLayer.

      function deposit(
          IERC20 token,
          uint256 tokenAmount
      ) external nonReentrant onlyRestakeManager returns (uint256 shares) {
          if (address(tokenStrategyMapping[token]) == address(0x0) || tokenAmount == 0) <---------
              revert InvalidZeroInput();
   
          // Move the tokens into this contract
          token.safeTransferFrom(msg.sender, address(this), tokenAmount);
   
          return _deposit(token, tokenAmount);
      }

4. The OperatorDelegator::deposit() function will end up reverting because it does not allow a token amount of 0 to be deposited

Mitigation Analysis

---

The mitigation proposed successfully fixes the issue by checking if _amount is greater than zero before approving the operator delegator to spend the collateral then depositing it. If it's equal to zero, it will not attempt to approve and deposit the collateral.

-        // Approve the tokens to the operator delegator
-        _collateralToken.safeApprove(address(operatorDelegator), _amount);

-        // Call deposit on the operator delegator
-        operatorDelegator.deposit(_collateralToken, _amount);

+        //  check if amount needs to be sent to operatorDelegator
+         if(_amount > 0) {
+             // Approve the tokens to the operator delegator
+             _collateralToken.safeApprove(address(operatorDelegator), _amount);

+             // Call deposit on the operator delegator
+             operatorDelegator.deposit(_collateralToken, _amount);
+         }

This ensures that no token amount _amount of value 0 will be supplied to OperatorDelegator::deposit() and therefore the function will no longer revert.

[c4-judge]

alcueca marked the issue as satisfactory