xiaoming90 - WETH is stuck in the router if users deposit or mint with Native ETH

[sherlock-admin2]

xiaoming90

high

WETH is stuck in the router if users deposit or mint with Native ETH

Summary

Certain routers' functions expect users to send Native ETH during deposit or minting. However, due to an error in the implementation, the deposited ETH will be stuck in the router, and subsequently, the stuck ETH can be stolen by others.

Vulnerability Detail

A total of two (2) instances of this issue were found.

Instance 1 - LMPVaultRouterBase.deposit

The LMPVaultRouterBase.deposit function is explicitly marked as payable and called the _processEthIn function to handle Native ETH. The LMPVaultRouterBase.deposit function expects users to send Native ETH during deposit as confirmed by the sponsor.

Assume that Alice granted max allowance to the LMPVaultRouterBase since she uses it frequently. Alice decided to deposit 100 Native ETH to the vault by calling the LMPVaultRouterBase.deposit function with 100 ETH attached to the TX (msg.value == 100 ETH).

When the LMPVaultRouterBase.deposit function is executed, 100 Native ETH is converted to 100 WETH within the LMPVaultRouterBase._processEthIn function and stored on the LMPVaultRouterBase contract.

Then, the code pulls an additional 100 WETH from the user address again at Line 54 below. So a total of 200 ETH is taken from Alice, but only 100 ETH of them is deposited to the LMPVault as per Line 56 below.

200 ETH are taken from Alice, but only 100 ETH worth of shares are minted to her, so she lost 100 ETH in this example.

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

File: LMPVaultRouterBase.sol
44:     function deposit(
45:         ILMPVault vault,
46:         address to,
47:         uint256 amount,
48:         uint256 minSharesOut
49:     ) public payable virtual override returns (uint256 sharesOut) {
50:         // handle possible eth
51:         _processEthIn(vault);
52: 
53:         IERC20 vaultAsset = IERC20(vault.asset());
54:         pullToken(vaultAsset, amount, address(this));
55: 
56:         return _deposit(vault, to, amount, minSharesOut);
57:     }
..SNIP..
111:     function _processEthIn(ILMPVault vault) internal {
112:         // if any eth sent, wrap it first
113:         if (msg.value > 0) {
114:             // if asset is not weth, revert
115:             if (address(vault.asset()) != address(weth9)) {
116:                 revert InvalidAsset();
117:             }
118: 
119:             // wrap eth
120:             weth9.deposit{ value: msg.value }();
121:         }
122:     }

The severity of this issue is aggravated by the fact that the LMPVaultRouterBase router contract inherits the PeripheryPayments contract, which contains a number of special functions (e.g. sweepToken, approve) that allow anyone to pull tokens that reside on the router contract.

Immediately after the transaction, malicious users/bots/MEV searchers can call the PeripheryPayments.sweepToken to obtain Alice's 100 WETH stuck in the LMPVaultRouterBase router, so her funds are lost forever.

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

File: PeripheryPayments.sol
58:     function sweepToken(IERC20 token, uint256 amountMinimum, address recipient) public payable {
59:         uint256 balanceToken = token.balanceOf(address(this));
60:         if (balanceToken < amountMinimum) revert InsufficientToken();
61: 
62:         if (balanceToken > 0) {
63:             token.safeTransfer(recipient, balanceToken);
64:         }
65:     }

Instance 2 - LMPVaultRouterBase.mint

The same issue occurs on the LMPVaultRouterBase.mint. In this case, the $x$ amount of Native ETH attached to the transaction will be ignored and not deposited to the vault. An additional assets amount of WETH will be pulled from the caller and deposited into the vault instead.

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

File: LMPVaultRouterBase.sol
23:     function mint(
24:         ILMPVault vault,
25:         address to,
26:         uint256 shares,
27:         uint256 maxAmountIn
28:     ) public payable virtual override returns (uint256 amountIn) {
29:         // handle possible eth
30:         _processEthIn(vault);
31: 
32:         IERC20 vaultAsset = IERC20(vault.asset());
33:         uint256 assets = vault.previewMint(shares);
34:         pullToken(vaultAsset, assets, address(this));
35:         vaultAsset.safeApprove(address(vault), assets);
36: 
37:         amountIn = vault.mint(shares, to);
38:         if (amountIn > maxAmountIn) {
39:             revert MaxAmountError();
40:         }
41:     }

Impact

Loss of assets as shown in the example above.

Code Snippet

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

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

Tool used

Manual Review

Recommendation

Ensure that the Native ETH (msg.value) sent to the function is handled properly if support for Native ETH is compulsory. Otherwise, it is recommended to remove the support of Native ETH to avoid any potential pitfalls.

function deposit(
    ILMPVault vault,
    address to,
    uint256 amount,
    uint256 minSharesOut
+ ) public virtual override returns (uint256 sharesOut) {
- ) 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);
}

function mint(
    ILMPVault vault,
    address to,
    uint256 shares,
    uint256 maxAmountIn
+ ) public virtual override returns (uint256 amountIn) {
- ) 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();
    }
}

Duplicate of #1