H-07 MitigationConfirmed

[c4-bot-2]

Lines of code

Vulnerability details

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

[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

---

After a withdrawal is queued by an admin by calling the function OperatorDelegator::queueWithdrawals(), the function completeQueuedWithdrawal() in the OperatorDelegator.sol is invoked by the admin onlyNativeEthRestakeAdmin EOA as a final step to finalize withdrawals.

The issue exists is in the completeQueuedWithdrawal() function. If the withdrawal contains ERC20 collateral tokens that are to be withdrawn, and there is a withdraw buffer for to-be-withdrawn ERC20 collateral token that needs to be filled, the function call will revert

function completeQueuedWithdrawal(
        IDelegationManager.Withdrawal calldata withdrawal,
        IERC20[] calldata tokens,
        uint256 middlewareTimesIndex
    ) external nonReentrant onlyNativeEthRestakeAdmin {
            .................
        for (uint256 i; i < tokens.length; ) {
            // check if token is not Native ETH
            if (address(tokens[i]) != IS_NATIVE) {
                // Check the withdraw buffer and fill if below buffer target
                uint256 bufferToFill = withdrawQueue.getBufferDeficit(address(tokens[i]));

                // get balance of this contract
                uint256 balanceOfToken = tokens[i].balanceOf(address(this));
                if (bufferToFill > 0) {
                    bufferToFill = (balanceOfToken <= bufferToFill) ? balanceOfToken : bufferToFill;
                    .
                    .
                    // fill Withdraw Buffer via depositQueue
        ----------> restakeManager.depositQueue().fillERC20withdrawBuffer(
                        address(tokens[i]),
                        bufferToFill
                    );
                    .
        }

If the token that is being checked isn't native ETH if (address(tokens[i]) != IS_NATIVE) and if there is a withdraw buffer for that ERC20 token that needs to be filled if (bufferToFill > 0), then the function DepositQueue::fillERC20withdrawBuffer() will be called.

    function fillERC20withdrawBuffer(
        address _asset,
        uint256 _amount
    ) external nonReentrant onlyRestakeManager <---------
    { 
        ..........
    }

The issue is that the fillERC20withdrawBuffer() function has the onlyRestakeManager modifier which allows only the RestakeManager.sol to call this function.

    /// @dev Allows only the RestakeManager address to call functions
    modifier onlyRestakeManager() {
        if (msg.sender != address(restakeManager)) revert NotRestakeManager();
        _;
    }

And since it's the OperatorDelegator calling this function, not the RestakeManager, this will revert and prevent the admins from completing any withdrawls that include an ERC20 collateral token that needs it's buffer to be filled.

Mitigation Analysis

---

The mitigation proposed for this issue successfully fixes it.

-    function fillERC20withdrawBuffer(
-         address _asset,
-         uint256 _amount
-     ) external nonReentrant onlyRestakeManager {
+ function fillERC20withdrawBuffer(address _asset, uint256 _amount) external nonReentrant

The mitigation simply removed the onlyRestakeManager modifier from the fillERC20withdrawBuffer() function which allows it to be called by the OperatorDelegator without any issues.

[c4-judge]

alcueca marked the issue as satisfactory