0x007 - LMPVaultRouterBase would still pull WETH token after processing ETH in

[sherlock-admin2]

0x007

high

LMPVaultRouterBase would still pull WETH token after processing ETH in

Summary

LMPVaultRouter inherits LMPVaultRouterBase and base asset is WETH. The mint and deposit functions are meant to work with either ETH or WETH. It ought to accept ETH and convert to WETH if user sends ETH (msg.value > 0). However, in this scenario, it would also pull WETH token.

Vulnerability Detail

As can be seen in the codebase, mint and deposit function would call _processEthIn and pullToken. _processEthIn would convert ETH to ERC20 WETH if msg.value > 0, while pullToken would transfer WETH from user address.

If users use WETH only, everything is fine. However, the contract is created so users have the choice to use ETH (instead of WETH). If such user send msg.value==amount and have WETH balance >= amount. The router would take both ETH and WETH while utilizing just the WETH. This would result in loss of asset to users. If they don't have WETH balance >= amount, the transaction would revert and thereby breaking a core feature.

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

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

POC

• user calls deposit with msg.value==amount==5ETH
• contract would convert the 5 ETH to 5 WETH in _processEthIn
• contract would pull 5 WETH from the user in pullToken
• That's 10 WETH in total
• But contract would deposit 5 WETH to vault
• Therefore user loses the first 5 ETH

Impact

Users will either lose their ETH or be unable to perform deposit with ETH.

Code Snippet

https://github.com/sherlock-audit/2023-06-tokemak/blob/main/v2-core-audit-2023-07-14/src/vault/LMPVaultRouterBase.sol#L22-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

if msg.value > 0, don't pull token. Either process eth or pull weth, but don't do both. Also update tests to include ETH mint and deposit.

Duplicate of #1