berndartmueller - Potentially overpaying for vault shares due to the lack of incorporating native ETH into the required assets amount

[sherlock-admin2]

berndartmueller

medium

Potentially overpaying for vault shares due to the lack of incorporating native ETH into the required assets amount

Summary

Using native ETH to mint vault shares or deposit into a vault does not incorporate the native ETH amount into the required assets amount of vaultAsset (i.e., WETH) tokens, resulting in overpaying for the vault shares.

Vulnerability Detail

Minting LMPVault shares using the LMPVaultRouterBase.mint function allows the caller to provide native ETH. The received ETH is wrapped to WETH via the _processEthIn function and kept within the router contract. Subsequently, in line 34 of the mint function, vaultAsset (i.e., WETH) tokens are attempted to be pulled in from the caller (msg.sender). However, if the caller has already provided native ETH, the additional pulled-in WETH token amount assets does not consider the previously wrapped ETH amount, resulting in overpaying for the vault shares.

Impact

If the caller (msg.sender) has approved the LMPVaultRouter contract with sufficient WETH token spending allowance, pulling in the additional WETH tokens will succeed. However, as twice the amount of assets is pulled in and only assets is utilized by the vault, the remaining funds will be kept in the router contract, up for grabs for MEV bots or other actors (using the sweepToken function of the PeripheryPayments child contract).

Otherwise, if the router's spending allowance is insufficient, the transaction will revert.

Code Snippet

Both the mint and deposit functions of the LMPVaultRouterBase contract suffer from the same issue.

src/vault/LMPVaultRouterBase.sol#L34

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));
36:
37:     amountIn = vault.mint(shares, to);
38:     if (amountIn > maxAmountIn) {
39:         revert MaxAmountError();
40:     }
41: }

src/vault/LMPVaultRouterBase.sol#L54

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: }

src/vault/LMPVaultRouterBase._processEthIn

The provided native ETH (i.e., msg.value) is wrapped to WETH and remains in the router contract.

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: }

src/utils/PeripheryPayments.pullToken

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

Tool used

Manual Review

Recommendation

Consider using the native ETH, which gets wrapped as WETH, provided by the caller and deduct this amount from the required assets amount. This results in less funds being pulled in from the caller, correctly using the native ETH provided by the caller.

Duplicate of #1