Description

There are two source of funds to deposit ETH in the contract: DELEGATED_STAKING and EETH. In DELEGATED_STAKING, the user could deposit the entire 32 ETH using the payable function batchDepositWithBidIds in StakingManager, which changes the phase of the validator ID to STAKE_DEPOSITED, and then call batchRegisterValidators, which registers the validator, mints them TNFT and BNFT, and changes the phase to LIVE. While in the case of EETH, the tokens are used from the liquidity pool and the phase is changed to WAITING_FOR_APPROVAL.

The two functions batchDepositWithBidIds and batchRegisterValidators are not required to be executed in the same transaction. Therefore, when the function batchDepositWithBidIds is called with the source of funds as DELEGATED_STAKING, the 32 ETH are stored in the StakingManager contract, which are later used in the batchRegisterValidators call. An attacker who has already called batchDepositAsBnftHolder, which internally calls batchDepositWithBidIds with the source of funds as EETH, could directly call the nonpayable batchRegisterValidators to get access to these 32 ETH, which are deposited by a legitimate user. The bug arises because an attacker could use these 32 ETH before the legitimate user calls batchRegisterValidators as this function does not check the source of these funds.

Impact

The malicious BNFT holder could get access to the ETH deposited by a legitimate user.

Recommendations

We recommend checking the source of funds in batchRegisterValidators.

Remediation

This issue has been acknowledged by EtherFi, and a fix was implemented in commit b2c2c31f↗.