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

0 stars 0 forks source link

M-09 MitigationConfirmed #53

Open c4-bot-6 opened 4 weeks ago

c4-bot-6 commented 4 weeks ago

Lines of code

Vulnerability details

C4 Issue

M-09: https://github.com/code-423n4/2024-04-renzo-findings/issues/198

Issue Details

In RestakeManager contract, function deposit() is used to deposit token to protocol. It checks the withdrawal buffer and fills it up using some or all of the deposited amount if it is below the buffer target. The remaining amount is then transferred to the operator delegator and deposited into EigenLayer.

function deposit(IERC20 _collateralToken, uint256 _amount, uint256 _referralId) public nonReentrant notPaused {
    // Verify collateral token is in the list - call will revert if not found
    uint256 tokenIndex = getCollateralTokenIndex(_collateralToken);
.  .  .

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

The issue is when bufferToFill > 0 and bufferToFill > amount, amount to deposit will become 0, and deposit() function in OperatorDelegator contract, it will revert on 0 deposit value:

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

Mitigation

The mitigation successfully mitigates the original issue by checking amount > 0 or not first, then call approve and call deposit() function in OperatorDelegator contract

Conclusion

Mitigation confirmed

c4-judge commented 4 weeks ago

alcueca marked the issue as satisfactory