sweepERC20() does not fill up withdrawal buffer

[howlbot-integration]

Lines of code

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

Vulnerability details

Impact

DepositQueue.sweepERC20() fails to call WithdrawQueue.getBufferDeficit() and DepositQueue.fillERC20withdrawBuffer() to fill up withdraw buffer deficit with available tokens before calling restakeManager.depositTokenRewardsFromProtocol() to deposit into Eigenlayer through the Operator Delegator contract.

The protocol usually fill withdraw buffer deficit for both 4th and Erc20 tokens as observed in; RM.deposit(), DQ.depositEthFromProtocol and DQ.forwardFullWithdrawalEth(). This is done to make Eth/tokens available for withdrawal by user.

However, when the sweep function was called, no call was made to fill any withdraw buffer deficit.

Having a withdraw deficit unfilled will reduce the amount of tokens that ought to be available for withdrawal.

// revert if amount to redeem is greater than withdrawBufferTarget
        if (amountToRedeem > getAvailableToWithdraw(_assetOut)) revert NotEnoughWithdrawBuffer();

Proof of Concept

  /// @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);
        }
    }

Also, no call in RM.depositTokenRewardsFromProtocol to check if there's a buffer deficit and to fill it.

Tools Used

Manual review.

Recommended Mitigation Steps

Include a withdrawal buffer deficit check and fill it if required.

Assessed type

Context

[C4-Staff]

CloudEllie marked the issue as primary issue

[c4-judge]

alcueca changed the severity to QA (Quality Assurance)

[c4-judge]

alcueca marked the issue as grade-a

[alcueca]

Useful suggestion, but hardly a Medium severity.

[c4-judge]

alcueca marked the issue as grade-b

[lanrebayode]

@alcueca Thanks for Judging ,

Considering the fact that, this discrepancy will make withdrawal unavailable to users when there should be, it temporarily DOS core functionality which IMO makes it a valid medium.

In this case, tokens that should be available for withdraw is being sent EL.

Please take a second look at this.

[s1n1st3r01]

@lanrebayode As mentioned in a comment in another report, sponsor pointed out numerous times that DepositQueue isn't supposed to be holding or receiving any ERC20 tokens. So what this report really is saying is "If some people donate ERC20 to the protocol, those ERC20 won't fill the withdraw buffer". This isn't a vulnerability but rather a great QA suggestion.

Therefore this claim:

this discrepancy will make withdrawal unavailable to users when there should be

is not true. The protocol doesn't rely on ERC20 donations to fill the withdraw buffer and make funds available to users for withdrawal by any means.

[alcueca]

s1n1st3r0 is right, while filling the withdraw buffer in depositTokenRewardsFromProtocol is a valid suggestion, there is no risk of lost assets or significant DoS by its absence.