warRoom - User can not deposit ETH in LMPVault via router without being at loss

[sherlock-admin]

warRoom

high

User can not deposit ETH in LMPVault via router without being at loss

Summary

deposit() function of the router collects ETH and WETH both while issuing the share for only one of them. If the user transfers eth and has also given the WETH approval for the deposit, the router will take both ETH and WETH, but it will only issue share for only one of them and in case user has only provided the ETH, the transaction will always revert because it will try to capture wETH as well which user has not approved of. Resulting in the user can only supply WETH to participate.

Vulnerability Detail

deposit() function of the LMPVaultRouterBase.sol tries to collect WETH and ETH from the user, with _processEthIn() Ether is deposited and then with pullToken() WETH is deposited.

LMPVaultRouterBase.deposit#L44

    function deposit(
        ILMPVault vault,
        address to,
        uint256 amount,
        uint256 minSharesOut
    ) public payable virtual override returns (uint256 sharesOut) {
        // handle possible eth
        _processEthIn(vault);

        IERC20 vaultAsset = IERC20(vault.asset());
        pullToken(vaultAsset, amount, address(this));
        return _deposit(vault, to, amount, minSharesOut);
    }

LMPVaultRouterBase._processEthIn#L111-L122

    function _processEthIn(ILMPVault vault) internal {
        // if any eth sent, wrap it first
        if (msg.value > 0) {
            // if asset is not weth, revert
            if (address(vault.asset()) != address(weth9)) {
                revert InvalidAsset();
            }

            // wrap eth
            weth9.deposit{ value: msg.value }();
        }
    }

PeripheryPayments.pullTokens#L54-L56

    function pullToken(IERC20 token, uint256 amount, address recipient) public payable {
        token.safeTransferFrom(msg.sender, recipient, amount);

Due to this User can not deposit ETH in LMPVault via router without being at a loss, Consider the following scenario :

1. Case 1: Alice Deposits only ETH

• In the first case, let's consider Alice who wants to deposit only ETH into the LMPVault. The process involves converting her ETH to WETH using the _processEthIn function, then these WETH tokens are sent to the Router contract.
• The problem arises in the next step when the pullToken() function is called. This function attempts to transfer the same amount of WETH tokens from Alice to the Router contract.
• However, this transfer fails because Alice hasn't given approval to the Router contract to access her WETH tokens. Consequently, the deposit of ETH for Alice also fails, preventing her from depositing ETH alone.

2. Case 2: Alice Deposits both ETH and WETH (Same amount)

• In the second case, Alice tries to deposit both ETH and WETH (equal amounts) into the LMPVault. The process remains the same initially: her ETH is converted to WETH using _processEthIn, and the resulting WETH tokens are sent to the Router contract.
• Again, the pullToken() function is executed, and this time the transfer of WETH tokens from Alice to the Router contract succeeds. This is because Alice has granted approval for the Router contract to access her WETH tokens. However, despite the successful deposit, Alice encounters a different issue.
• The LMPVault calculates Alice's share based on either her deposited ETH or WETH. Consequently, she only receives a share equivalent to half of what she initially deposited (either in ETH or WETH). Additionally, Alice's funds become stuck within the Router contract
• This will happen when Alice has given a huge amount of WETH approval to the router previously and then she tries to deposit with ETH.

Impact

The user won't be able to deposit ETH without incurring a loss, So DOS or loss of funds.

Code Snippet

https://github.com/sherlock-audit/2023-06-tokemak/blob/main/v2-core-audit-2023-07-14/src/vault/LMPVaultRouterBase.sol#L44-L57

https://github.com/sherlock-audit/2023-06-tokemak/blob/main/v2-core-audit-2023-07-14/src/vault/LMPVaultRouterBase.sol#L111-L122

https://github.com/sherlock-audit/2023-06-tokemak/blob/main/v2-core-audit-2023-07-14/src/utils/PeripheryPayments.sol#L54-L56

Tool used

Manual Review

Recommendation

Take isETH bool as the argument from a user specifying whether they want to supply ETH or not, then based on that separate the logic.

Duplicate of #1