EETHDepositHelper Manual Review Findings

EET-01M: Inexistent Initialization Protection of Base Implementation

Type	Severity	Location
Language Specific		EETHDepositHelper.sol:L20

Description:

The contract is meant to be upgradeable yet does not properly protect its logic deployment from malicious initializations.

Example:

Copy
16contract EETHDepositHelper is OwnableUpgradeable {
17    IeETH private _eETH;
18    IweETH private _weETH;
19
20    function initialize(IeETH eETH_, IweETH weETH_) public initializer {
21        __Ownable_init();
22
23        _eETH = eETH_;
24        _weETH = weETH_;
25    }

Recommendation:

We advise a constructor to be introduced that either invokes the initializer modifier of the Initializable contract or invokes the Initializable::_disableInitializers function to prevent the base implementation from ever being initialized.

Alleviation (58e8cc66dfa900c03c47df78f5170d9960005629):

An EETHDepositHelper::constructor has been introduced invoking the Initializable::initialize modifier thereby preventing re-initializations as long as the contract does not utilize a versioned initialization system.

If such a system is expected, we advise the Initializable::_disableInitializers function instead.

EET-02M: Potentially Insecure Approval Operations

Type	Severity	Location
Logical Fault		EETHDepositHelper.sol:L63, L66, L72

Description:

The referenced approval operations utilize deprecated functions by OpenZeppelin that may result in transaction failures in case the consumers of the approvals do not consume them in full.

We can confirm that such a case may occur by inspecting BasicVault::deposit which may not consume the full allowance depending on the Cap::addLoad result.

Impact:

Once the BasicVault reaches its Cap limitation, any further deposit operation will fail to execute properly due to an approval failure.

Example:

Copy
61function _deposit(uint256 amount, address vault) internal {
62    SafeERC20.safeTransferFrom(_eETH, _msgSender(), address(this), amount);
63    SafeERC20.safeIncreaseAllowance(_eETH, address(_weETH), amount);
64    uint256 weETHAmount = _weETH.wrap(amount);
65
66    SafeERC20.safeIncreaseAllowance(_weETH, address(vault), weETHAmount);
67    IVault(vault).deposit(amount, _msgSender());
68}

Recommendation:

We advise the SafeERC20::forceApprove function to be utilized instead, ensuring that the approval operation will go through, and the approval to be erased after the respective operation by using the same function with a zero (0) input argument.

Alleviation (58e8cc66dfa900c03c47df78f5170d9960005629):

The code was refactored to simultaneously perform a SafeERC20::forceApprove operation and to evaluate the actual amount of funds the deposit would require, ensuring that a precise value is approved.

We consider this exhibit alleviated as a result of these actions.