Renzo - umarkhatab_465's results

Lines of code

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

Vulnerability details

Impact

Having no slippage params on deposit functions will lead to following scenario

• receiving less ezETH tokens when the transaction is executed at times when the ezETH exchanged is less than the amount desired.

Proof of Concept

As of this report being written 3 May 2024 5:38 PM the price of ezETH is 2,975$ however , just 9 minutes ago the price was below 2930$. It can be seen that price is fluctuating but the price of eth right now is 3,014$

So if the transaction initiated by the user falls in the time frame let's say

t1-t2 , user might have bought the eth at 2930$ but if the transaction has been executed at say between this time frame

t3-t4

the user might have paid 3000$ for a RenzoETH which presents a loss scenario for user.

so user might want to add slippage parameters like

minEzAmountOut or deadline parameters so that when the transaction is executed , its executed at a suitable price and satisfy the risk appetite of the investor. Otherwise the transaction should fail.

However , the deposit functions in the RestakeManager contract does not allow the user to flexibly provide these parameters to the contract functions to either accept the execution or reject.

Most probably users would incur loss and we always want to minimize user's loss. The counter situation of user making profit is understandable but that's a good thing . We want to minimize bad things here.

   function deposit(
        IERC20 _collateralToken,
        uint256 _amount,
        uint256 _referralId
    ) public nonReentrant notPaused { }
 function deposit(IERC20 _collateralToken, uint256 _amount) external {
        deposit(_collateralToken, _amount, 0);
    }
  function depositETH() external payable {
    }

    function depositETH(uint256 _referralId) public payable nonReentrant notPaused {}

Tools Used

Manual Review , Special Manual Review Tool

Recommended Mitigation Steps

Add slippage params in the deposit functions like deadline and minAmountOut

Assessed type

Timing

Lines of code

https://github.com/code-423n4/2024-04-renzo/blob/main/contracts/Withdraw/WithdrawQueue.sol#L206

Vulnerability details

Impact

The withdraw method in WithdrawQueue accepts the assetOut address

and asset amount to place order for .

However , it fails to account for slippage scenarios due to calculating of assetOut

amount to be returned by an oracle and the prices returned by oracles vary

plus there can be scenarios when the transactions are just delayed due to network

congestion or other factors which might lead users to placing orders of receiving

less assetOut than anticipated at the time of transaction submission .

Proof of Concept

Here is the code for withdraw

 function withdraw(uint256 _amount, address _assetOut) external nonReentrant {
        // check for 0 values
        if (_amount == 0 || _assetOut == address(0)) revert InvalidZeroInput();

        // check if provided assetOut is supported
        if (withdrawalBufferTarget[_assetOut] == 0) revert UnsupportedWithdrawAsset();

        // transfer ezETH tokens to this address
        IERC20(address(ezETH)).safeTransferFrom(msg.sender, address(this), _amount);

        // calculate totalTVL
        (, , uint256 totalTVL) = restakeManager.calculateTVLs();

        // Calculate amount to Redeem in ETH
        uint256 amountToRedeem = renzoOracle.calculateRedeemAmount(
            _amount,
            ezETH.totalSupply(),
            totalTVL
        );

        // update amount in claim asset, if claim asset is not ETH
        if (_assetOut != IS_NATIVE) {
            // Get ERC20 asset equivalent amount
            amountToRedeem = renzoOracle.lookupTokenAmountFromValue(
                IERC20(_assetOut),
                amountToRedeem
            );
        }

        // revert if amount to redeem is greater than withdrawBufferTarget
        if (amountToRedeem > getAvailableToWithdraw(_assetOut)) revert NotEnoughWithdrawBuffer();

        // increment the withdrawRequestNonce
        withdrawRequestNonce++;

        // add withdraw request for msg.sender
        withdrawRequests[msg.sender].push(
            WithdrawRequest(
                _assetOut,
                withdrawRequestNonce,
                amountToRedeem,
                _amount,
                block.timestamp
            )
        );

        // add redeem amount to claimReserve of claim asset
        claimReserve[_assetOut] += amountToRedeem;

        emit WithdrawRequestCreated(
            withdrawRequestNonce,
            msg.sender,
            _assetOut,
            amountToRedeem,
            _amount,
            withdrawRequests[msg.sender].length - 1
        );
    }

Take following scenario .

1. Alice submits a transaction of withdraw amount 100 input tokens with current tokenA being returned , according to market value , to be 100
2. The transaction gets delayed due to several factors i.e network congestion.
3. When the the transaction is executed , the price has surged and now the exchange

of 100 input tokens to 5 output tokens.

4. The transaction gets executed and Alice incurs a loss worth of 95 output tokens.

The loss escalates when the price surge is high and a lot of assetOut is involved

like 2 billion.

Due to lack of minimum amount out and deadline parameter withdraw

the user has incurred a significant loss of out tokens.

Tools Used

Manual review

Recommended Mitigation Steps

Add slippage parameters like minAmountOut and deadline

and only initiates withdrawal requests if

amountToBeRedeemed >= minAmountOut

and

block.timestamp<=deadline

otherwise, revert the withdraw order.

Assessed type

Timing