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

0 stars 0 forks source link

M-09 MitigationConfirmed #26

Open c4-bot-7 opened 3 months ago

c4-bot-7 commented 3 months ago

Lines of code

Vulnerability details

See:

Finding Mitigation
M-09: Deposits will always revert if the amount being deposited is less than the bufferToFill value Pull Request

Navigating to M-09 from the previous contest we can see that there was an issue within the deposit function in the RestakeManager contract that enables users to deposit ERC20 whitelisted collateral tokens.

The crux of the issue was the fact that if the deposited amount is less than the buffer deficit, the entire deposit is used to fill the withdrawal buffer, leaving a zero amount for further operations which then leads to a problem when the execution tries to deposit this zero amount to the operator delegator which essentially reverts the transaction cause the OperatorDelegator's deposit function does not accept zero amounts if (address(tokenStrategyMapping[token]) == address(0x0) || tokenAmount == 0) revert InvalidZeroInput();. This issue then causes user deposits to fail if the deposit amount is less than the buffer deficit.

Now, to mitigate this issue, protocol used this pull request which sufficiently mitigates this issue considering the deposit function has now been modified to only approve and transfer the amount to the operator delegator if it is greater than zero, preventing the function from attempting to deposit zero amounts and thus ensuring successful deposits, i.e

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

Now, whereas the changes are not correctly tagged with the pull request, I'm still tagging this as mitigation confirmed, considering from this pull request that was finalized to mitigate H-08 & M-09 from the previous contest we can see the complete implementation of the new changes

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

        // Get the TVLs for each operator delegator and the total TVL
        (
            uint256[][] memory operatorDelegatorTokenTVLs,
            uint256[] memory operatorDelegatorTVLs,
            uint256 totalTVL
        ) = calculateTVLs();

        // Get the value of the collateral token being deposited
        uint256 collateralTokenValue = renzoOracle.lookupTokenValue(_collateralToken, _amount);

        // Enforce TVL limit if set, 0 means the check is not enabled
        if (maxDepositTVL != 0 && totalTVL + collateralTokenValue > maxDepositTVL) {
            revert MaxTVLReached();
        }

        // Enforce individual token TVL limit if set, 0 means the check is not enabled
        if (collateralTokenTvlLimits[_collateralToken] != 0) {
            // Track the current token's TVL
            uint256 currentTokenTVL = 0;

            // For each OD, add up the token TVLs
            uint256 odLength = operatorDelegatorTokenTVLs.length;
            for (uint256 i = 0; i < odLength; ) {
                currentTokenTVL += operatorDelegatorTokenTVLs[i][tokenIndex];
                unchecked {
                    ++i;
                }
            }

            // Check if it is over the limit
            if (currentTokenTVL + collateralTokenValue > collateralTokenTvlLimits[_collateralToken])
                revert MaxTokenTVLReached();
        }

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

        // Transfer the collateral token to this address
        _collateralToken.safeTransferFrom(msg.sender, address(this), _amount);

-        // 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);
+         }
        // Calculate how much ezETH to mint
        uint256 ezETHToMint = renzoOracle.calculateMintAmount(
            totalTVL,
            collateralTokenValue,
            ezETH.totalSupply()
        );

        // Mint the ezETH
        ezETH.mint(msg.sender, ezETHToMint);

        // Emit the deposit event
        emit Deposit(msg.sender, _collateralToken, _amount, ezETHToMint, _referralId);
    }
c4-judge commented 3 months ago

alcueca marked the issue as satisfactory