zzykxx - ethToeETH() returns the amount of shares minted instead of the amount of tokens received

[sherlock-admin2]

zzykxx

high

ethToeETH() returns the amount of shares minted instead of the amount of tokens received

Summary

The function _ethTOeEth(), used to convert ETH into eETH, incorrectly returns the amount of shares minted instead of the amount of tokens received. This results in the protocol staking less eETH than it should, with the remaining part being stuck in the contract.

Vulnerability Detail

The function _ethTOeEth() deposits ETH in the EtherFi protocol in exchange for eETH tokens:

function _ethTOeEth(uint256 _amount) internal returns (uint256) {
   return IeETHLiquidityPool(eETHLiquidityPool).deposit{value: _amount}(address(this)); //@5 this returns `shares` but should return `balance`
}

The value returned by the deposit() function is the amount of shares received as it can be seen from the implementation:

function deposit(address _referral) public payable whenNotPaused returns (uint256) {
    ...SNIP...
    return _deposit(msg.sender, msg.value, 0);
}

function _deposit(address _recipient, uint256 _amountInLp, uint256 _amountOutOfLp) internal returns (uint256) {
    totalValueInLp += uint128(_amountInLp);
    totalValueOutOfLp += uint128(_amountOutOfLp);
    uint256 amount = _amountInLp + _amountOutOfLp;
    uint256 share = _sharesForDepositAmount(amount);
    if (amount > type(uint128).max || amount == 0 || share == 0) revert InvalidAmount();

    eETH.mintShares(_recipient, share);

    return share;
}

This is an issue because both depositWeth() and depositEth() expect the return value of _ethTOeEth() to represent the amount of eETH received instead of the amount of shares received.

The amount returned by _ethTOeEth() will be lower than the amount of eETH received (because shares are worth more), which results in the caller depositing less eETH than expected with the remaining unstaked eETH being stuck in the contract.

POC

Alice wants to deposit 1ETH:

1. Alice calls depositEth() by sending 1 ETH
2. _ethTOeEth() deposits 1 ETH in Etherfi which returns the amount of shares deposited, 962364978556513677.
3. depositEth() stakes 962364978556513677 eETH in the Sophon protocol.
4. The remaining 1e18 - 962364978556513677 is stuck in the contract.

Alice wanted to stake 1 ETH, but only ~0.9623ETH got staked.

Impact

At the current valuation of EtherFi shares about 3.7% of funds deposited in the weETH pool via either depositEth() or depositWeth() will not be staked and will be stuck in the contract.

Code Snippet

Tool used

Manual Review

Recommendation

In _ethTOeEth() return the balance received instead of the amount of shares:

function _ethTOeEth(uint256 _amount) internal returns (uint256) {
    // deposit returns exact amount of eETH
    uint256 balanceBefore = IERC20(eETH).balanceOf(address(this));
    IeETHLiquidityPool(eETHLiquidityPool).deposit{value: _amount}(address(this)); 
    return (IERC20(eETH).balanceOf(address(this)) - balanceBefore);
}

Duplicate of #4

[sherlock-admin2]

1 comment(s) were left on this issue during the judging contest.

0xmystery commented:

valid because it's the shares that matter

[sherlock-admin2]

The protocol team fixed this issue in the following PRs/commits:
sophon-org/farming-contracts@73adb67

[sherlock-admin2]

The Lead Senior Watson signed off on the fix.