Lines of code

https://github.com/code-423n4/2024-04-renzo/blob/519e518f2d8dec9acf6482b84a181e403070d22d/contracts/RestakeManager.sol#L491 https://github.com/code-423n4/2024-04-renzo/blob/519e518f2d8dec9acf6482b84a181e403070d22d/contracts/RestakeManager.sol#L592

Vulnerability details

The deposit() & depositETH functions of the RestakeManager contract, enables users to deposit assets into the protocol, getting ezETH tokens in return. The function doesn't have any type of slippage control; this is relevant in the context of the deposit function, since the amount of tokens received by the user is determined by an interaction with an oracle, meaning that the amount received in return may vary indefinitely while the request is waiting to be executed.

Also the users will have no defense against price manipulations attacks, if they where to be found after the protocol's deployment.

Proof of Concept

As can be observed by looking at it's parameters and implementation, the deposit function of the RestakeManager contract, doesn't have any type of slippage protection:

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

        .....

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

        // 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
            ///@audit-issue M 3 transfers happening this will be problematic for stETH, maybe this will never work with stETH and stETH pools remain empty 
            depositQueue.fillERC20withdrawBuffer(address(_collateralToken), bufferToFill);
        }
        ///@audit-ok isn't this is wrong because if(bufferToFill>0) then bufferToFill tokens are already transferred to depositQueue - all good because amount is getting reduced `_amount -= bufferToFill`
        ///> maybe putting this inside a if() statement will be good, I mean safeApprove & deposit should only run when `amount > 0`
        ///@audit-issue M if() is important here otherwise when amount will be reduced to zero, this whole trx will revert due to the tokenAmount zero check present in the OD's deposit! `InvalidZeroInput`
        // Approve the tokens to the operator delegator
        _collateralToken.safeApprove(address(operatorDelegator), _amount);

        // Call deposit on the operator delegator
        operatorDelegator.deposit(_collateralToken, _amount);

        ///@audit-ok calculating mint amount independent of the asset type? - lookupTokenValue gives collateralTokenValue based on the asset type
        // Calculate how much ezETH to mint
        uint256 ezETHToMint = renzoOracle.calculateMintAmount(
            totalTVL,
            collateralTokenValue,
            ezETH.totalSupply()
        );
        ///@audit-issue M slippage check missing!
        // Mint the ezETH
        ezETH.mint(msg.sender, ezETHToMint);

        // Emit the deposit event

 emit Deposit(msg.sender, _collateralToken, _amount, ezETHToMint, _referralId);
    }

Impact

Users have no control over how many ezETH tokens they will get in return for depositing in the contract.

Tools Used

Shaheen's Vision

Recommended Mitigation Steps

And additional parameter minAmount could be added to the deposit function, to let users decide the minimum amount of tokens to be received, with a relative check after minting.

-    function deposit(IERC20 _collateralToken, uint256 _amount, uint256 _referralId) public nonReentrant notPaused {
+    function deposit(IERC20 _collateralToken, uint256 _amount, uint256 _referralId, uint256 minAmt) 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);

        .....

        // Calculate how much ezETH to mint
        uint256 ezETHToMint = renzoOracle.calculateMintAmount(
            totalTVL,
            collateralTokenValue,
            ezETH.totalSupply()
        );
        // Mint the ezETH
+       require(ezETHToMint >= minAmt, "Slippage Protection");
        ezETH.mint(msg.sender, ezETHToMint);

        // Emit the deposit event
        emit Deposit(msg.sender, _collateralToken, _amount, ezETHToMint, _referralId);
    }

Assessed type

Other

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

Lack of slippage control for users on deposit functions #347