Usage discrepancy between ETH and ERC20 rewards in withdraw buffer

[howlbot-integration[bot]]

Lines of code

https://github.com/code-423n4/2024-04-renzo/blob/main/contracts/Deposits/DepositQueue.sol#L252-L277

Vulnerability details

Impact

There's a discrepancy in how rewards are utilized between Ethereum (ETH) and ERC20 tokens regarding filling the withdrawal buffer.

Proof of Concept

1. ETH Rewards Handling: When ETH rewards are received, a portion of the received ETH is automatically transferred to the Withdraw Queue to fill any buffer deficit. This is evident in the receive() function of the Deposit Queue contract.

   receive() external payable nonReentrant {
       uint256 feeAmount = 0;
       // Take protocol cut of rewards if enabled
       if (feeAddress != address(0x0) && feeBasisPoints > 0) {
           feeAmount = (msg.value * feeBasisPoints) / 10000;
           (bool success, ) = feeAddress.call{ value: feeAmount }("");
           if (!success) revert TransferFailed();
   
           emit ProtocolFeesPaid(IERC20(address(0x0)), feeAmount, feeAddress);
       }
       // update remaining rewards
       uint256 remainingRewards = msg.value - feeAmount;
       // Check and fill ETH withdraw buffer if required
       _checkAndFillETHWithdrawBuffer(remainingRewards);
   
       // Add to the total earned
       totalEarned[address(0x0)] = totalEarned[address(0x0)] + remainingRewards;
   
       // Emit the rewards event
       emit RewardsDeposited(IERC20(address(0x0)), remainingRewards);
   }

   https://github.com/code-423n4/2024-04-renzo/blob/main/contracts/Deposits/DepositQueue.sol#L158-L183

    /**
     * @notice  Check if WithdrawBuffer Needs to be filled
     */
    function _checkAndFillETHWithdrawBuffer(uint256 _amount) internal {
        // Check the withdraw buffer and fill if below buffer target
        uint256 bufferToFill = withdrawQueue.getBufferDeficit(IS_NATIVE);

        if (bufferToFill > 0) {
            bufferToFill = (_amount <= bufferToFill) ? _amount : bufferToFill;

            // fill withdraw buffer from received ETH
            withdrawQueue.fillEthWithdrawBuffer{ value: bufferToFill }();

            emit BufferFilled(IS_NATIVE, bufferToFill);
        }
    }

https://github.com/code-423n4/2024-04-renzo/blob/main/contracts/Deposits/DepositQueue.sol#L291-L306

2. ERC20 Rewards Handling: However, when ERC20 tokens are received as rewards, they are transferred to the Restake Manager contract through sweepERC20, and subsequently deposited into the strategy. However, there's no check to fill the withdrawal buffer using ERC20 rewards.

   /// @dev Sweeps any accumulated ERC20 tokens in this contract to the RestakeManager
   /// Only callable by a permissioned account
   function sweepERC20(IERC20 token) external onlyERC20RewardsAdmin {
       uint256 balance = IERC20(token).balanceOf(address(this));
       if (balance > 0) {
           uint256 feeAmount = 0;
   
           // Sweep fees if configured
           if (feeAddress != address(0x0) && feeBasisPoints > 0) {
               feeAmount = (balance * feeBasisPoints) / 10000;
               IERC20(token).safeTransfer(feeAddress, feeAmount);
   
               emit ProtocolFeesPaid(token, feeAmount, feeAddress);
           }
   
           // Approve and deposit the rewards
           token.approve(address(restakeManager), balance - feeAmount);
           restakeManager.depositTokenRewardsFromProtocol(token, balance - feeAmount);
   
           // Add to the total earned
           totalEarned[address(token)] = totalEarned[address(token)] + balance - feeAmount;
   
           // Emit the rewards event
           emit RewardsDeposited(IERC20(address(token)), balance - feeAmount);
       }
   }

   https://github.com/code-423n4/2024-04-renzo/blob/main/contracts/Deposits/DepositQueue.sol#L252-L277

    /// @dev Deposit ERC20 token rewards from the Deposit Queue
    /// Only callable by the deposit queue
    function depositTokenRewardsFromProtocol(
        IERC20 _token,
        uint256 _amount
    ) external onlyDepositQueue {
        // Get the TVLs for each operator delegator and the total TVL
        (, uint256[] memory operatorDelegatorTVLs, uint256 totalTVL) = calculateTVLs();

        // Determine which operator delegator to use
        IOperatorDelegator operatorDelegator = chooseOperatorDelegatorForDeposit(
            operatorDelegatorTVLs,
            totalTVL
        );

        // Transfer the tokens to this address
        _token.safeTransferFrom(msg.sender, address(this), _amount);

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

        // Deposit the tokens into EigenLayer
        operatorDelegator.deposit(_token, _amount);
    }

https://github.com/code-423n4/2024-04-renzo/blob/main/contracts/RestakeManager.sol#L645-L668

The protocol correctly fills the withdrawal buffer with ETH rewards but fails to utilize ERC20 rewards to fill the buffer. This inconsistency could lead to inefficient management of withdrawal buffers, potentially causing imbalance issues.

Tools Used

Recommended Mitigation Steps

To ensure consistent handling of rewards and prevent imbalance issues, modify the depositTokenRewardsFromProtocol function to check and fill the withdrawal buffer with ERC20 rewards similar to ETH rewards.

    function depositTokenRewardsFromProtocol(
        IERC20 _token,
        uint256 _amount
    ) external onlyDepositQueue {
        // Get the TVLs for each operator delegator and the total TVL
        (, uint256[] memory operatorDelegatorTVLs, uint256 totalTVL) = calculateTVLs();

        // Determine which operator delegator to use
        IOperatorDelegator operatorDelegator = chooseOperatorDelegatorForDeposit(
            operatorDelegatorTVLs,
            totalTVL
        );

        // Transfer the tokens to this address
        _token.safeTransferFrom(msg.sender, address(this), _amount);

        uint256 bufferToFill = depositQueue.withdrawQueue().getBufferDeficit(
            address(_token)
        );
        if (bufferToFill > 0) {
            bufferToFill = (_amount <= bufferToFill) ? _amount : bufferToFill;
            // update amount to send to the operator Delegator
            _amount -= bufferToFill;

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

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

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

        // Deposit the tokens into EigenLayer
        operatorDelegator.deposit(_token, _amount);
    }

Assessed type

Context

[C4-Staff]

CloudEllie marked the issue as duplicate of #322

[c4-judge]

alcueca changed the severity to QA (Quality Assurance)

[c4-judge]

alcueca marked the issue as grade-a