Inaccurate WETH and `xezETH` accounting inside `xRenzoBridge::xReceive()` due to absence of try/catch block

[howlbot-integration[bot]]

Lines of code

https://github.com/code-423n4/2024-04-renzo/blob/main/contracts/Bridge/L1/xRenzoBridge.sol#L175-L193 https://github.com/code-423n4/2024-04-renzo/blob/main/contracts/Bridge/L2/xRenzoDeposit.sol#L434

Vulnerability details

Impact

The xReceive() function inside xRenzoBridge.sol does not follow the official recommendation applicable in case of a revert while making an external call i.e. using a try/catch block while making external calls.
It's important to note the following statement made in the Connext docs:

If the call on the receiver contract (also referred to as "target" contract) reverts, funds sent in with the call will end up on the receiver contract.

It then goes on to say that:

To avoid situations where user funds get stuck on the receivers, developers should build any contract implementing IXReceive defensively. Ultimately, the goal should be to handle any revert-susceptible code and ensure that the logical owner of funds always maintains agency over them.

Hence, when xRenzoDeposit::sweep() makes a xcall to xRenzoBridge::xReceive() with balance, the following happens:

• balance is passed into the _amount param here.
• xRenzoBridge::xReceive() internally calls restakeManager.depositETH() which is followed by wrapping of ezETH into xezETH via the lockbox and then burning the xezETH.
• If any of these transactions involving external calls revert for any reason, for example if the restakeManager.depositETH() call reverts, then as per the comments in the Connext documentation the _amount WETH continues to remain in the contract. There is no provision now for an admin to retry the transaction and ensure that a correct amount of xezETH is burned and unwrapped WETH deposited into the restakeManager.
  Although the admin can next use recoverERC20() to recover the WETH, the xezETH accounting remains unattended to.

Proof of Concept

• https://docs.connext.network/developers/guides/handling-failures#reverts-on-receiver-contract

Tools Used

Manual review

Recommended Mitigation Steps

As has been officially recommended, enclose these external call inside xRenzoBridge::xReceive() in a try/catch block which logs the failure of the transaction. Also add an admin controlled function which can then be called to retry the deposit & burn steps.

Assessed type

Other

[C4-Staff]

CloudEllie marked the issue as primary issue

[jatinj615]

The try/catch won't mitigate anything. WETH will be present in the xRenzoBridgeContract and the xReceive can be triggered again through connext.

[c4-judge]

alcueca marked issue #373 as primary and marked this issue as a duplicate of 373

[c4-judge]

alcueca marked the issue as satisfactory

[alcueca]

The try/catch would send the tokens at a better place to recover them, at least.