Renzo - Maroutis's results

Lines of code

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

Vulnerability details

Impact

The incorrect usage of an array index in the RestakeManager::calculateTVLs function leads to wrong calculations of the TVL for withdrawal queues. Using the wrong index results in referencing the wrong token for value lookup, thereby potentially inflating or deflating the actual TVL calculated. The erroneous totalTVL returned by the function RestakeManager::calculateTVLs is used by many functions such as RestakeManager::deposit, WithdrawQueue::withdraw and BalancerRateProvider::getRate which can cause many issues down the line.

Proof of Concept

The issue arises in the following segment of the RestakeManager::calculateTVLs function :

                // record token value of withdraw queue
                if (!withdrawQueueTokenBalanceRecorded) {
                    totalWithdrawalQueueValue += renzoOracle.lookupTokenValue(
                        collateralTokens[i],//@audit using i index
                        collateralTokens[j].balanceOf(withdrawQueue)
                    );

As this calculation only runs for the first i loop (i=0), it will use the oracle of the first token to convert the balance of every single available token which is wrong. This leads to a wrong calculation of totalTVL returned by the function.

This has many impacts, I will list some of them :

• It can cause the RestakeManager::deposit function to not work as expected. Some checks could be or not bypassed (or the opposite revert when it should not), and the mint value will be wrong (since totalTVL is wrong):

...
        // Enforce TVL limit if set, 0 means the check is not enabled
        if (maxDepositTVL != 0 && totalTVL + collateralTokenValue > maxDepositTVL) {
            revert MaxTVLReached();
        }
...
        // Determine which operator delegator to use
        IOperatorDelegator operatorDelegator = chooseOperatorDelegatorForDeposit(
            operatorDelegatorTVLs,
            totalTVL
        );
...
        uint256 ezETHToMint = renzoOracle.calculateMintAmount(
            totalTVL,
            collateralTokenValue,
            ezETH.totalSupply()
        );

• In the WithdrawQueue contract the redeem amount will be wrong

// function withdraw
...
        (, , uint256 totalTVL) = restakeManager.calculateTVLs();

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

• Finally, in the BalancerRateProvider contract the rate returned will be wrong

// function getRate()

    function getRate() external view returns (uint256) {
        // Get the total TVL priced in ETH from restakeManager
        (, , uint256 totalTVL) = restakeManager.calculateTVLs();

        // Get the total supply of the ezETH token
        uint256 totalSupply = ezETHToken.totalSupply();

        // Sanity check
        if (totalSupply == 0 || totalTVL == 0) revert InvalidZeroInput();

        // Return the rate
        return (10 ** 18 * totalTVL) / totalSupply;

Tools Used

Manual review

Recommended Mitigation Steps

Replace the index i with the correct index j in the problematic line:

totalWithdrawalQueueValue += renzoOracle.lookupTokenValue(
    collateralTokens[j],
    collateralTokens[j].balanceOf(withdrawQueue)
);

Assessed type

Error

Lines of code

https://github.com/code-423n4/2024-04-renzo/blob/519e518f2d8dec9acf6482b84a181e403070d22d/contracts/Bridge/L2/Oracle/RenzoOracleL2.sol#L13 https://github.com/code-423n4/2024-04-renzo/blob/519e518f2d8dec9acf6482b84a181e403070d22d/contracts/Bridge/L2/Oracle/RenzoOracleL2.sol#L52

Vulnerability details

Impact

The stale period 86400 + 60 seconds used for the oracle price validation is too short for some tokens like ezETH for example on Arbitrum. This could lead to the protocol consuming stale prices on Arbitrum.

Proof of Concept

In both RenzoOracle and RenzoOracleL2, the hearbeat period MAX_TIME_WINDOW is set to 86400 + 60; // 24 hours + 60 seconds. In the functions RenzoOracleL2::getMintRate and RenzoOracle::lookupTokenValue, a validation checks sees if the price data fed by Chainlink's price feed aggregators is stale depending if the period of 24 hours + 60 seconds has passed. Example :

    function getMintRate() public view returns (uint256, uint256) {
        (, int256 price, , uint256 timestamp, ) = oracle.latestRoundData();
        if (timestamp < block.timestamp - MAX_TIME_WINDOW) revert OraclePriceExpired();

The problem is that depending on the token and the chain, the same period can be considered too small or too stale.

Let's consider the ezETH/ETH oracles on different chains:

• On Ethereum, the oracle will update the price data every ~24 hours.
• On Arbitrum, the oracle will update the price data every ~6 hours.
• On Linea, the oracle will update the price data every 24 hours. ...

This means that on Arbitrum, 24 hours can be considered too large for the stale period which will cause the function RenzoOracleL2::getMintRate to return stale data.

Tools Used

Manual review

Recommended Mitigation Steps

It is recommanded to store a mapping that would record the hearbeat parameter for the stale period of each token and for every different chain.

Assessed type

Oracle

Lines of code

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

Vulnerability details

Impact

The protocol's RestakeManager::deposit function lacks adequate slippage protection when minting ezETH to users. This will leave users vulnerable to MEV bots and price manipulation.

Proof of Concept

In the RestakeManager::deposit function, the mint amount for ezETH is calculated based on the current TVL and the value of the newly added collateral. However, due to price fluctuations caused by MEV bots, the actual amount of ezETH tokens returned by the function renzoOracle.calculateMintAmount and received by the user might be significantly lower than expected, leading to financial losses.

//RestakeManager::deposit
...
        uint256 ezETHToMint = renzoOracle.calculateMintAmount(
            totalTVL,
            collateralTokenValue,
            ezETH.totalSupply()
        );

Tools Used

Manual review

Recommended Mitigation Steps

It is recommended to implement slippage protection by requiring users to provide a minimu amount parameter when calling the RestakeManager::deposit function. After calculating the amount of ezEth to mint, the function should check if this amount meets the user's specified minimum threshold, protecting them from potential losses due to price manipulation.

Assessed type

MEV