Breeje - User can pay double amount in Vault Router and Excess amount can be stolen

[sherlock-admin2]

Breeje

medium

User can pay double amount in Vault Router and Excess amount can be stolen

Summary

In case, the vault's asset is weth and user wants to pay in eth which is accepted by the contract, user can end up forced to pay twice. Furthermore, this excess amount can be stolen by anyone.

Vulnerability Detail

Same vulnerability exists in both mint and deposit functions in LMPVaultRouterBase contract. So focusing on just mint function for details.

mint function in LMPVaultRouterBase contract is a publicly access function which any user can use to mint shares of any vault by depositing the base asset of that vault.

File: LMPVaultRouterBase.sol

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

Link to Code

Here's the scenario of how User can end up paying double the asset:

1. Assume Alice wants to mint shares of a vault whose underlying base asset is weth.

2. Alice sends the 2 ethers while calling mint function to get it's equivalent of shares from the vault.

3. The contract will call _processEthIn(vault) to check whether the underlying asset is weth or not.

File: LMPVaultRouterBase.sol

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

4. As in this scenario, vault.asset() is weth9, the contract will deposit to weth9 to convert it to weth.

5. Next, total assets to be send will be calculated using previewMint and again, the tokens are pulled from the user to the contract worth of assets value (marked with @-> above).

As Alice has already sent the eth, taking assets' value worth of weth again is a redundent call which results in loss of funds for Alice.

Also, there is a public function unwrapWETH9:

File: PeripheryPayments.sol

  function unwrapWETH9(uint256 amountMinimum, address recipient) public payable { // @audit-info Can Steal any Weth from the router
      uint256 balanceWETH9 = weth9.balanceOf(address(this));

      if (balanceWETH9 < amountMinimum) revert InsufficientWETH9();

      if (balanceWETH9 > 0) {
          weth9.withdraw(balanceWETH9);
          Address.sendValue(payable(recipient), balanceWETH9);
      }
  }

So Anyone can call this function to steal the Excess weth contract owns because of double transfer from Alice.

Impact

Loss of Funds for Users

Code Snippet

Shown Above

Tool used

Manual Review

Recommendation

Update the codebase as shown below:

File: LMPVaultRouterBase.sol

  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)); 
+       if(msg.value == 0){
+           pullToken(vaultAsset, assets, address(this));
+       } else if(msg.value < assets){
+           pullToken(vaultAsset, assets - msg.value, address(this));
+       }
        vaultAsset.safeApprove(address(vault), assets);

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

    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)); 
+       if(msg.value == 0){
+           pullToken(vaultAsset, amount, address(this));
+       } else if(msg.value < amount){
+           pullToken(vaultAsset, amount - msg.value, address(this));
+       }

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

Duplicate of #1