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

11 stars 8 forks source link

L2 deposits can bypass the `maxTVL` check #393

Closed howlbot-integration[bot] closed 5 months ago

howlbot-integration[bot] commented 5 months ago

Lines of code

https://github.com/code-423n4/2024-04-renzo/blob/519e518f2d8dec9acf6482b84a181e403070d22d/contracts/Bridge/L2/xRenzoDeposit.sol#L168-L191 https://github.com/code-423n4/2024-04-renzo/blob/519e518f2d8dec9acf6482b84a181e403070d22d/contracts/Bridge/L1/xRenzoBridge.sol#L139-L201 https://github.com/code-423n4/2024-04-renzo/blob/519e518f2d8dec9acf6482b84a181e403070d22d/contracts/RestakeManager.sol#L592-L616

Vulnerability details

Impact

Other chains where xEzETH minted collects the ETH and admin sweeps it to L1. When funds received in L1, funds are deposited into restake manager to get ezETH. However, minting ezETH in L2 does not care about the max TVL in restake manager but the L1 does. When funds are bridged, if the TVL limit is reached then the sweeping will fail and ezETH in L1 will not be minted. This will lead to existence of ezETH in other chains that are not backed in L1. Also, if the maximum tvl is reached in L1 anyone can keep minting ezETH in L2's bypassing the tvl limit.

Proof of Concept

Users can deposit ETH/WETH to other chains deposit contract to receive xezETH which can be exchanged to ezETH in mainnet by connext bridge.

When funds are gathered in L2 deposit contract, admin can call the sweep function to send ETH/WETH to L1 bridge contract where the ETH/WETH will be deposited to restake manager to get ezETH.

As we can see in the xReceive function in the bridge contract of L1 which is the function will be called by connext when funds are sweeped from L2 to L1, the funds are deposited to restaking manager in bulk.

function xReceive(
        bytes32 _transferId,
        uint256 _amount,
        address _asset,
        address _originSender,
        uint32 _origin,
        bytes memory
    ) external nonReentrant returns (bytes memory) {
        .
        .
        // Deposit it into Renzo RestakeManager
        -> restakeManager.depositETH{ value: ethAmount }();
        .
        .
        .
    }

However, as we can see in restake manager depositETH function, if the max TVL is reached the function can revert:

function depositETH(uint256 _referralId) public payable nonReentrant notPaused {
        .
        // Enforce TVL limit if set
        -> if (maxDepositTVL != 0 && totalTVL + msg.value > maxDepositTVL) {
            revert MaxTVLReached();
        }
        .
        .
    }

If the function revert because of the maxDepositTVL is reached, then the sweep function will fail. However, the xEzETH that is minted by the L2 users can still bridge their L2 ezETH to L1 ezETH but since the L1 ezETH is not minted because of maxTVL the bridged xEzETH will have no collateral backing in L1.

Also, users will be able to mint xezETH in L2 although the maxTVL is reached in L1, breaking one of the core functionality of the protocol

Tools Used

Recommended Mitigation Steps

Send the maximum tvl limit to L2 via oracle just like price.

Assessed type

Other

jatinj615 commented 5 months ago

MaxTVL feature is not being used in the protocol anymore. Can be removed.

c4-judge commented 5 months ago

alcueca changed the severity to 2 (Med Risk)

c4-judge commented 5 months ago

alcueca marked the issue as duplicate of #287

c4-judge commented 5 months ago

alcueca marked the issue as satisfactory

c4-judge commented 4 months ago

alcueca marked the issue as duplicate of #373