code-423n4 / 2024-04-renzo-findings

9 stars 7 forks source link

Users will be unable to deposit ERC20 collateral if the amount they want to deposit <= bufferToFill #187

Closed howlbot-integration[bot] closed 4 months ago

howlbot-integration[bot] commented 4 months ago

Lines of code

https://github.com/code-423n4/2024-04-renzo/blob/519e518f2d8dec9acf6482b84a181e403070d22d/contracts/Delegation/OperatorDelegator.sol#L147 https://github.com/code-423n4/2024-04-renzo/blob/1c7cc4e632564349b204b4b5e5f494c9b0bc631d/contracts/RestakeManager.sol#L546-L549 https://github.com/code-423n4/2024-04-renzo/blob/1c7cc4e632564349b204b4b5e5f494c9b0bc631d/contracts/RestakeManager.sol#L562

Vulnerability details

Impact

A denial of service will allways occur for users who's deposit amount happens to match the needed bufferToFill which will cause them not being able deposit or mint ezETH tokens.

Proof of Concept

When users deposit a collateral token, the required calculations are performed, the amount is pulled from the caller and the buffer deficit is gotten from withdrawQueue.sol. Now if bufferToFill is greater than zero, the bufferToFill is set to a new value. If _amount <= bufferToFill, the bufferToFill is capped to the amount, upon which it is reduced by bufferToFill. This causes the now _amount to be 0. This 0 _amount is then granted as allowance to OperatorDelegator.sol upon which the deposit function is called.

    function deposit(
        IERC20 _collateralToken,
        uint256 _amount,
        uint256 _referralId
    ) public nonReentrant notPaused {
        ...

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

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

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

The deposit function in OperatorDelegator.sol checks if tokenAmount is 0, which if it is, will revert, causing the deposit function to revert.

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

This causes that users will not be able to deposit, mint ezETH and the buffer deficit will not be clearable through the deposit function.

Tools Used

Manual code review

Recommended Mitigation Steps

Wrap the operatorDelegator.deposit function in an if() block.

    function deposit(
        IERC20 _collateralToken,
        uint256 _amount,
        uint256 _referralId
    ) public nonReentrant notPaused {
        ...

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

Assessed type

Error

c4-judge commented 4 months ago

alcueca marked the issue as satisfactory