vagrant - wETH deposited/minted via LMPVaultRouterBase is taken twice from msg.sender if he chooses to deposit with msg.value

[sherlock-admin]

vagrant

high

wETH deposited/minted via LMPVaultRouterBase is taken twice from msg.sender if he chooses to deposit with msg.value

Summary

LMPVaultRouterBase offers the user the option to send ETH with his calls to mint/deposit. This msg.value will be deposited into the wETH contract yet it will not be used for minting vault shares, instead the contract pulls the tokens required for minting vault shares from the user again, making him pay twice if he has wETH in his wallet. The wETH stranded in the contract can be easily stolen by anyone.

Vulnerability Detail

LMPVaultRouterBase mint() and deposit():
https://github.com/sherlock-audit/2023-06-tokemak/blob/main/v2-core-audit-2023-07-14/src/vault/LMPVaultRouterBase.sol#L23

    function mint(
        ILMPVault vault,
        address to,
        uint256 shares,
        uint256 maxAmountIn
    ) public payable virtual override returns (uint256 amountIn) {
        // handle possible eth
        _processEthIn(vault);

        IERC20 vaultAsset = IERC20(vault.asset());
        uint256 assets = vault.previewMint(shares);
        pullToken(vaultAsset, assets, address(this)); // @audit why do we need to pull tokens from the user if he already sent ETH with the transaction?
        vaultAsset.safeApprove(address(vault), assets);

        amountIn = vault.mint(shares, to);
        if (amountIn > maxAmountIn) {
            revert MaxAmountError();
        }
    }

    /// @inheritdoc ILMPVaultRouterBase
    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)); // @audit same here

        return _deposit(vault, to, amount, minSharesOut);
    }

    /// @dev Assumes tokens are already in the router
    function _deposit(
        ILMPVault vault,
        address to,
        uint256 amount,
        uint256 minSharesOut
    ) internal returns (uint256 sharesOut) {
        approve(IERC20(vault.asset()), address(vault), amount);
        if ((sharesOut = vault.deposit(amount, to)) < minSharesOut) {
            revert MinSharesError();
        }
    }

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

    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 }();
        }
    }

as you can see, when minting/depositing, the contract first converts the msg.value into wETH if the vault.asset is wETH.
The contract receives this wETH but it does not use it, instead it pulls the vaultAsset directly from the msg.sender via pullToken:

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

The LMPVaultRouterBase inherits from PeripheryPayments which has function that lets anyone steal the wETH in the contract:

    function sweepToken(IERC20 token, uint256 amountMinimum, address recipient) public payable {
        uint256 balanceToken = token.balanceOf(address(this));
        if (balanceToken < amountMinimum) revert InsufficientToken();

        if (balanceToken > 0) {
            token.safeTransfer(recipient, balanceToken);
        }
    }

Impact

possible loss of funds for users because funds might be pulled twice from the user

Code Snippet

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

Tool used

Manual Review

Recommendation

use the wETH that the contract receives from depositing into wETH contract to mint shares if the vaultAsset is wETH instead of pulling the tokens from the user

Duplicate of #1