Users will loose some or all of their Deposits

[c4-bot-1]

Lines of code

https://github.com/code-423n4/2024-04-renzo/blob/519e518f2d8dec9acf6482b84a181e403070d22d/contracts/Bridge/L1/xRenzoBridge.sol#L139-L202

Vulnerability details

Impact

Users will loose some of their deposits

Proof of Concept

The xRenzoBridge.sol accepts users collateral from the bridge and deposits it into Renzo using the xReceive() The xReceive() function takes all the collateral and deposit it into Renzo The major problem is that an attacker can disrupt this whole process by sending ether directly into the bridge contract as the calculations regarding how much to deposit for user is given as

        uint256 ethAmount = address(this).balance - ethBalanceBeforeWithdraw;

Lets say User A intends on depositing 1 ether, before xReceive() is executed, the attacker directly sends in 0.5 or 1 ether into the xRenzoBridge contract. This will indeed cause the user to loose half or all of their ether because the amount that is gonna be sent to the Restaking manager contract is the ethAmount

  restakeManager.depositETH{ value: ethAmount }();

Over time this will lead to accumulated ether in the contract that will unbalance other users expected deposit amount. Leading to a bad user experience overall.

Tools Used

Manual Review

Recommended Mitigation Steps

Tho the protocol implements a sweep function that periodically sweeps accidental ETH value sent to the contract, alot of damage would have been done before the sweep function will be called. A more efficient method will be to include a minimum deposit amount parameter to allow to set the minimum deposit amount that will be deposited per deposit and in a situation whereby it is less than the minimum amount set, then the whole transaction will revert. This will be a more forward approach.

Assessed type

Other

[DadeKuma]

No, this is just a way to calculate the delta from unwrapping WETH, and the logic is correct.

[DadeKuma]

@howlbot reject

[okolicodes7]

Hi good day The report above highlights how a user could lose the Eth collateral they intend on depositing into Renzo if Eth is directly sent into the xRenzoBridge.sol. The report explains in detail how a user can loose their intended deposit as you can see above. The function accepts Eth collateral deposit and calculates the intended deposit amount of the user which can be wrongly calculated if Eth is sent directly to the contract as explained above. The recommended fix is the best approach to solving this issue.

[DadeKuma]

Tho the protocol implements a sweep function that periodically sweeps accidental ETH value sent to the contract, alot of damage would have been done before the sweep function will be called.

Here: https://github.com/code-423n4/2024-04-renzo/blob/519e518f2d8dec9acf6482b84a181e403070d22d/contracts/Bridge/L1/xRenzoBridge.sol#L294