Invalid check in `LiquidityPool.withdraw` function prevents users from withdrawing their deposits

[hats-bug-reporter[bot]]

Github username: -- Twitter username: -- Submission hash (on-chain): 0x23b9bcafba7e25f79d43c2ff27224438035b75268999ab03888b375b5589354f Severity: medium

Description: Description\ Describe the context and the effect of the vulnerability.

Attack Scenario

• LiquidityPool contract depends on users depositing their ethers and getting eETH share tokens as a source of liquidity/funds.

• When a user deposits in the LP; he will be minted eETH shares proportional to the totalSupply of the shares tokens (eETH.totalShares()) and the available ether balance of the pool (totalPooledEther):

LiquidityPool._deposit function

  ```solidity
  function _deposit() internal returns (uint256) {
        totalValueInLp += uint128(msg.value);
        uint256 share = _sharesForDepositAmount(msg.value);
        if (msg.value > type(uint128).max || msg.value == 0 || share == 0) revert InvalidAmount();

        eETH.mintShares(msg.sender, share);

        return share;
  }
  ```

LiquidityPool._sharesForDepositAmount function

  ```solidity
  function _sharesForDepositAmount(uint256 _depositAmount) internal view returns (uint256) {
        uint256 totalPooledEther = getTotalPooledEther() - _depositAmount;
        if (totalPooledEther == 0) {
              return _depositAmount;
        }
        return (_depositAmount * eETH.totalShares()) / totalPooledEther;
  }
  ```

• And since the pool can receive ethers from any one directly (by direct transfer of ethers without going through the deposit process); so the eETH.totalShares() and the totalPooledEther are not necessarily 1:1.

• for example:

  1. alice is the first depositor,she deposits 1 ether in the liquidity pool(where the pool initial ether balance is zero); so she will be minted 1e18 of eETH, and the eETH:ETH is 1:1 (1e18 shares are minted for 1 ether -which equals 1e18-).
  2. then the pool receives 2 ethers directly via receive function; so the total balance of the pool is now 3 ethers.
  3. then bob deposits in the pool 1 ether; and he will be minted (_depositAmount * eETH.totalShares()) / totalPooledEther which equals to : 1e18*1e18/3e18 = 0.33e18 = 33e16 of eETH share tokens.

• Any user wants to withdraw his asset from the pool, he must first add a withdrawal request; then withdraws.

Mitigation

LiquidityPool.withdraw function

    function withdraw(address _recipient, uint256 _amount) external whenNotPaused returns (uint256) {
        uint256 share = sharesForWithdrawalAmount(_amount);
        require(msg.sender == address(withdrawRequestNFT) || msg.sender == address(membershipManager), "Incorrect Caller");
-        if (totalValueInLp < _amount || (msg.sender == address(withdrawRequestNFT) && -ethAmountLockedForWithdrawal < _amount) || eETH.balanceOf(msg.sender) < _amount) revert InsufficientLiquidity();

+        if (totalValueInLp < _amount || (msg.sender == address(withdrawRequestNFT) && +ethAmountLockedForWithdrawal < _amount) || eETH.balanceOf(msg.sender) < share) revert InsufficientLiquidity();
        if (_amount > type(uint128).max || _amount == 0 || share == 0) revert InvalidAmount();

        totalValueInLp -= uint128(_amount);
        if (msg.sender == address(withdrawRequestNFT)) {
            ethAmountLockedForWithdrawal -= uint128(_amount);
        }

        eETH.burnShares(msg.sender, share);

        (bool sent, ) = _recipient.call{value: _amount}("");
        if (!sent) revert SendFail();

        return share;
    }

[seongyun-ko]

This statement is false:

"And since the pool can receive ethers from any one directly (by direct transfer of ethers without going through the deposit process); so the eETH.totalShares() and the totalPooledEther are not necessarily 1:1."

The totalPooledEther does not change by any direct ETH transfer to the LiquidityPool contract.

[seongyun-ko]

close