Renzo - honey-k12's results

Lines of code

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

Vulnerability details

Impact

calculateTVLs() fails to consider ETH queued for withdrawals when withdrawals are initiated from sources external to Renzo. This oversight leads to an underestimated TVL, potentially enabling an attacker to purchase ezETH at a temporary discount.

Proof of Concept

The problem lies in the fact that withdrawals may be triggered from outside of Renzo. This can happen when:

1. Operator who holds active key initiates validator exit
2. Validator is slashed to 16 ETH and is forcefully exited
3. Operator calls undelegate on Eigenlayer's DelegationManager

When withdrawals are initiated from external sources, such as above, Renzo's TVL calculation fails to account for the ETH queued for withdrawal. This oversight results in an underestimation of TVL during the withdrawal delay period, creating an opportunity for individuals to purchase ezETH at a temporary discount.

Consider this hypothetical situation:

1. The total supply of ezETH and TVL is 96 ETH, implying a ratio of 1 ezETH to 1 ETH.
2. A malicious operator holding a minority allocation 'accidentally' triggers a withdrawal on the Beacon chain by undelegating himself.
3. The staked ETH balance decreases by 32 ETH, reflecting on Eigenlayer and reducing the shares of the Eigenpod by 32 ETH.
4. Renzo's calculateTVLs() decreases by 32 ETH due to the reduction in shares, but Renzo remains unaware of the ETH in the withdrawal queue. Consequently, the exchange rate shifts to 96 ezETH to 64 ETH, resulting in a ratio of 0.67.
5. The malicious operator capitalizes on the opportunity to purchase cheap ezETH. They wait for the withdrawal to complete, after which the ETH is transferred to Renzo's Restaking Manager, restoring the TVL to its original value.
6. The value of ezETH increases, allowing the operator to sell ezETH for a profit.

A malicious operator or any observer who identifies this issue can profit by purchasing ezETH at a lower price and selling it for a profit several days later. This comes at the expense of existing ezETH holders, whose shares will be diluted.

This scenario could also occur during a significant slashing event, such as a smart contract bug, where numerous validators are slashed to 16 ETH and forcefully exited. Observers of this event would have an opportunity to exploit the temporary underpricing of ezETH.

Tools Used

Manual Review

Recommended Mitigation Steps

Consider off-chain monitoring for such edge case withdrawals and temporarily pause Renzo operations until the withdrawals have completed and ETH is back inside Renzo's system. Will require implementation of a pause function in Renzo.

Assessed type

Other

Lines of code

https://github.com/code-423n4/2024-04-renzo/blob/519e518f2d8dec9acf6482b84a181e403070d22d/contracts/Withdraw/WithdrawQueue.sol#L220-L224 https://github.com/code-423n4/2024-04-renzo/blob/519e518f2d8dec9acf6482b84a181e403070d22d/contracts/Withdraw/WithdrawQueue.sol#L242-L249

Vulnerability details

Proof of Concept

In case of penalties Eigenlayer can be notified of the balance drop via the permissionless function EigenPod::verifyBalanceUpdates(). In case of slashing the validator is forced to exit and Eigenlayer can be notified via the permissionless function EigenPod::verifyAndProcessWithdrawals() because the slashing event is effectively a full withdrawal.

Changes to the actual balance of validators are updated when verifyBalanceUpdates() is called on an EigenPod. Slashing of an EigenPod is therefore known before it is updated in the EigenLayer system.

A user can monitor the validators and call withdraw() when a validator is slashed to front-run calls to verifyBalanceUpdates() in order to avoid the loss.

If a user calls withdraw() before the balance is update on EigenLayer they will inflate amountToRedeem they are entitled to since the TVL has still not accounted for the slashing that happened.

WithdrawQueue.sol#L220-L224

        uint256 amountToRedeem = renzoOracle.calculateRedeemAmount(
            _amount,
            ezETH.totalSupply(),
            totalTVL
        );

This is saved in the withdraw request for user

WithdrawQueue.sol#L242-L249

        withdrawRequests[msg.sender].push(
            WithdrawRequest(
                _assetOut,
                withdrawRequestNonce,
                amountToRedeem,
                _amount,
                block.timestamp
            )

The user will increase their amountToRedeem since the TVL is inflated.

Impact

Example scenario:

Initial state: 2 validators 32 ETH each 10 users with equal LRT, 6.4 each.

Validator 1 is slashed for 16 ETH User 1 front-runs verifyBalanceUpdates() with a call to withdraw() and has amountToRedeem = 6.4 since TVL is still 64 ETH. verifyBalanceUpdates() is now called to update EigenLayer balance. User 2 calls withdraw() and has amountToRedeem= 4.8 since TVL has decreased to 48. User 1 has stolen 1.4 ETH from the rest.

Thus, slashing will be concentrated among those that don't front-run. When this issue becomes known a new "meta" will be at play, everybody will have to front-run otherwise they risk losing all their funds since they will have to cover an outsized part of the slashing penalty.

Tools Used

Manual Review

Recommended Mitigation Steps

Adjust the withdrawal process to account for penalties or slashing events ensuring fair distribution of penalties among all stakers.

Assessed type

Other

Lines of code

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

Vulnerability details

Impact

The incorrect calculation of totalWithdrawalQueueValue leads to a false calculation of the total TVL for the protocol. This breaks the Main invariant of the protocol: ezETH should be minted or redeemed based on current supply and TVL

Proof of Concept

calculateTVLs() function calculates the Total Value Locked (TVL) for each operator delegator by individual token, total for each operator delegator, and total for the protocol. RestakeManager.sol#L270-L274

    function calculateTVLs() public view returns (uint256[][] memory, uint256[] memory, uint256) {

It also calculates the withdrawalQueue total for each tokwn value by looping through the collateralTokens array RestakeManager.sol#L286-L287

        // withdrawalQueue total value
       uint256 totalWithdrawalQueueValue = 0;

However, there is a vulnerability in the calculation of totalWithdrawalQueueValue, where the wrong token value is retrieved due to incorrect indexing, leading to an inaccurate calculation of the total TVL.

RestakeManager.sol#L299-L326

Inside the inner loop, When the withdrawQueueTokenBalanceRecorded flag is false, following code block executes: RestakeManager.sol#L316-L321

                if (!withdrawQueueTokenBalanceRecorded) {
                    totalWithdrawalQueueValue += renzoOracle.lookupTokenValue(
                        collateralTokens[i],
                        collateralTokens[j].balanceOf(withdrawQueue)
                    );
                }

Here, i is the index for iterating over operatorDelegators, and j is the index for iterating over collateralTokens. However, in the call to lookupTokenValue, collateralTokens[i] is used instead of collateralTokens[j]. This means that the function is looking up the value of the wrong token, specifically the token at the index of the current operator delegator(i=0), not the current token.

This leads to the wrong token value being retrieved, resulting in an incorrect calculation of totalWithdrawalQueueValue.

Furthermore, withdrawQueueTokenBalanceRecorded flag is set to true after the first iteration of the outer loop. Therefore, the code block inside the if (!withdrawQueueTokenBalanceRecorded) condition is only executed during the first iteration (when i=0). For subsequent iterations (when i>0), this code block is skipped. RestakeManager.sol#L344

            withdrawQueueTokenBalanceRecorded = true;

The incorrect calculation of totalWithdrawalQueueValue leads to a false calculation of the total TVL for the protocol. RestakeManager.sol#L352

        totalTVL += address(depositQueue).balance;

It breaks the Main invariant of the protocol: ezETH should be minted or redeemed based on current supply and TVL.

Total TVL is a critical metric and is used in various functions within the protocol:

RestakeManager.sol::deposit#L500-L504

        (
            uint256[][] memory operatorDelegatorTokenTVLs,
            uint256[] memory operatorDelegatorTVLs,
            uint256 totalTVL
        ) = calculateTVLs();

RestakeManager.sol::depositETH()#L594

        (, , uint256 totalTVL) = calculateTVLs();

RestakeManager.sol::depositTokenRewardsFromProtocol()#L652

    (, uint256[] memory operatorDelegatorTVLs, uint256 totalTVL) = calculateTVLs();

WithdrawQueue.sol::withdraw()#L217

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

Tools Used

Manual Review

Recommended Mitigation Steps

if (!withdrawQueueTokenBalanceRecorded) {
    totalWithdrawalQueueValue += renzoOracle.lookupTokenValue(
        collateralTokens[j],
        collateralTokens[j].balanceOf(withdrawQueue)
    );
}

In this corrected code, lookupTokenValue is called with the correct token (collateralTokens[j]), and the balance of that token in the withdrawal queue. This ensures that the total value of the withdrawal queue is calculated correctly, using the correct prices for each token.

Assessed type

Error

Lines of code

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

Vulnerability details

Impact

The deposit() function of the RestakeManager.sol 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 its parameters and implementation, the deposit() function of the RestakeManager.sol contract, doesn’t have any type of slippage protection: RestakeManager.sol#L473

    function deposit(IERC20 _collateralToken, uint256 _amount) external {

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

The amount of tokens to be minted, with respect to the deposited amount, is determined by the calculateMintAmount function

RestakeManager.sol#L565-L569

        uint256 ezETHToMint = renzoOracle.calculateMintAmount(
            totalTVL,
            collateralTokenValue,
            ezETH.totalSupply()
        );

The values of the tokens being deposited is determined as follows:

RestakeManager.sol#L507

        uint256 collateralTokenValue = renzoOracle.lookupTokenValue(_collateralToken, _amount);

The lookupTokenValue function queries the external oracle for the asset price:

RenzoOracle.sol#L71-L81

    function lookupTokenValue(IERC20 _token, uint256 _balance) public view returns (uint256) {
        AggregatorV3Interface oracle = tokenOracleLookup[_token];
        if (address(oracle) == address(0x0)) revert OracleNotFound();


        (, int256 price, , uint256 timestamp, ) = oracle.latestRoundData();
        if (timestamp < block.timestamp - MAX_TIME_WINDOW) revert OraclePriceExpired();
        if (price <= 0) revert InvalidOraclePrice();


        // Price is times 10**18 ensure value amount is scaled
        return (uint256(price) * _balance) / SCALE_FACTOR;
    }

Thus, there is no protection for the user in the current implementation. Meaning that the user has no way to predict how many ezETH they will get back at the moment of minting, as the price could be updated while the request is in the mempool.

Tools Used

Manual Review

Recommended Mitigation Steps

An additional parameter 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.

Assessed type

Other

Lines of code

https://github.com/code-423n4/2024-04-renzo/blob/519e518f2d8dec9acf6482b84a181e403070d22d/contracts/Oracle/RenzoOracle.sol#L60-L62

Vulnerability details

Impact

If a depositor stakes stETH, the value of their asset could fluctuate. This issue affects not only the staked asset but also all existing stETH tokens in the protocol.

Proof of Concept

Currently, ezETH is minted based on the eth feeds for assets. RenzoOracle.sol::L60-62

        // Verify that the pricing of the oracle is 18 decimals - pricing calculations will be off otherwise
        if (_oracleAddress.decimals() != 18)
            revert InvalidTokenDecimals(18, _oracleAddress.decimals());

RestakeManager.sol::L491-495

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

        uint256 collateralTokenValue = renzoOracle.lookupTokenValue(_collateralToken, _amount);

RenzoOracle.sol#L71-L81

    function lookupTokenValue(IERC20 _token, uint256 _balance) public view returns (uint256) {
        AggregatorV3Interface oracle = tokenOracleLookup[_token];
        if (address(oracle) == address(0x0)) revert OracleNotFound();


        (, int256 price, , uint256 timestamp, ) = oracle.latestRoundData();
        if (timestamp < block.timestamp - MAX_TIME_WINDOW) revert OraclePriceExpired();
        if (price <= 0) revert InvalidOraclePrice();


        // Price is times 10**18 ensure value amount is scaled
        return (uint256(price) * _balance) / SCALE_FACTOR;
    }

However, this could be an issue for stETH because its USD feed is more reliable than its ETH feed.

While using the ASSET/ETH feed is a safe choice for some assets, it could pose a problem for assets with safer USD feeds. For instance, the stETH token has two Chainlink-supported feeds:

The stETH/ETH feed at 0x86392dC19c0b719886221c78AB11eb8Cf5c52812 The stETH/USD feed at 0xCfE54B5cD566aB89272946F602D76Ea879CAb4a8

On the official Chainlink site, we can see that the former feed has a heartbeat of of a significant 86400 seconds, while the latter has a heartbeat of just 3600 seconds.

Chainlink-stEth-priceFeeds

This means the price can change up to 24 hours before a price update is triggered on the stETH/ETH feed, resulting in a significant difference between the on-chain price and the actual stETH price. On the other hand, the stETH/USD feed updates every hour, making it more accurate as it stays close to the actual stETH price.

This is particularly important when minting ezETH, as the stETH price is often queried. In some cases, it might even be queried twice, especially when the provided asset is stETH. This is because the value of the tokens provided by the depositor and the assets held by the protocol are both determined in relation to this price. This could potentially double the impact of any discrepancies in the price. Therefore, it’s crucial to consider these factors when dealing with stETH.

Tools Used

VS Code

Recommended Mitigation Steps

Implement two feeds to get the price of stETH, i.e the stETH/USD feed, then use the ETH/USD feed to convert the price to ETH.

Assessed type

Oracle