nobody2018 - LMPVaultRouterBase.mint/deposit will make the user's weth be stolen in some cases

[sherlock-admin]

nobody2018

medium

LMPVaultRouterBase.mint/deposit will make the user's weth be stolen in some cases

Summary

LMPVault.asset() is baseAsset(WETH). mint/deposit can send native token (ether) in exchange for vault shares due to the payable modifier. They all call [_processEthIn](https://github.com/sherlock-audit/2023-06-tokemak/blob/main/v2-core-audit-2023-07-14/src/vault/LMPVaultRouterBase.sol#L111-L122) internally to wrap Ether to WETH, and then call [pullToken](https://github.com/sherlock-audit/2023-06-tokemak/blob/main/v2-core-audit-2023-07-14/src/utils/PeripheryPayments.sol#L54-L56) to transfer WETH from the caller to the router.

There is a common case: most users who frequently interact with UnsiwapRouter will approve some tokens to UnsiwapRouter (token.approve(router, someBigAmount)).

If the user has approved WETH to LMPVaultRouter before and wants to use native token to call mint/deposit, this may cause the sent native token to be stuck in the contract (wrapped as WETH). Anyone can steal WETH through [LMPVaultRouter.sweepToken](https://github.com/sherlock-audit/2023-06-tokemak/blob/main/v2-core-audit-2023-07-14/src/utils/PeripheryPayments.sol#L58-L65) (inherited from PeripheryPayments).

Vulnerability Detail

For the sake of brevity, here we only analyze deposit, and mint is similar.

File: v2-core-audit-2023-07-14\src\vault\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));//@audit vaultAsset.safeTransferFrom(msg.sender, this, amount);
55: 
56:         return _deposit(vault, to, amount, minSharesOut);
57:     }
......
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:     }

L51, if msg.value > 0, _processEthIn(vault) will wrap Ether to WETH.

L54, pullToken will transfer from the caller to WETH to the router.

If msg.value = 0, this is no problem.

If msg.value = amount, then the caller's WETH will be silently transferred to the router. This is equivalent to the caller consuming amount*2 funds, but the amount is left in the router. As a result, anyone who notices this can steal WETH via the following function:

File: v2-core-audit-2023-07-14\src\utils\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:     }

Impact

In the above case, the user's WETH may be quietly sent to the router, resulting in a 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#L23-L57

Tool used

Manual Review

Recommendation

Whether pullToken(vaultAsset, assets, address(this)) is called depends on msg.value:

1. If msg.value == 0, then call it.
2. If msg.value > 0, then do not call it, and check whether msg.value is equal to the required amount of baseAsset.

Duplicate of #1