Skip to content
This repository has been archived by the owner on Mar 3, 2024. It is now read-only.

harisnabeel - User can lost funds when mint() with ETH from LMPVaultRouter #732

Closed
sherlock-admin opened this issue Aug 30, 2023 · 0 comments
Labels
Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label High A valid High severity issue Reward A payout will be made for this issue

Comments

@sherlock-admin
Copy link
Contributor

sherlock-admin commented Aug 30, 2023

harisnabeel

high

User can lost funds when mint() with ETH from LMPVaultRouter

Summary

When minting with ETH, LMPVaultRouterBase's mint() function also pull WETH from caller if the caller had approved WETH back in some time.

Vulnerability Detail

1 - Suppose the ratio in vault is 1:1, Alice wants to mint 1e18 shares, as WETH and ETH are 1:1, so inside mint() of LMPVaultRouterBase _processEthIn() converts deposited ETH into WETH. The functions will be called with params like below.

mint(wethVaultAddress,userAddress,1e18,1e18)

2 - Assuming some users who have WETH and had approved to LMPVaultRouter back in some time. In spite of getting ETH, following line also pulls 1 WETH from user.

        pullToken(vaultAsset, assets, address(this));

3 - The user will get shares of 1 ETH, and will lose 1 WETH unknowingly.

The WETH pulled from user will remain inside the LMPVaultRouter unless someone/BOT calls sweepToken() and gets those WETH.

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

Impact

Those users who have WETH in their account and had approved WETH to LMPVaultRouter will lose WETH when minting with ETH.

Code Snippet

   /// @inheritdoc ILMPVaultRouterBase
      function mint(
        ILMPVault vault,
        address to,
        uint256 shares,
        uint256 maxAmountIn
    ) public payable virtual override returns (uint256 amountIn) {
        // handle possible eth
        _processEthIn(vault); // @audit

        IERC20 vaultAsset = IERC20(vault.asset());
        uint256 assets = vault.previewMint(shares);
        pullToken(vaultAsset, assets, address(this)); // @audit
        vaultAsset.safeApprove(address(vault), assets);

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

Tool used

Manual Review

Recommendation

Only pull WETH from user when msg.value < maxAmountIn.
Make sure to pull the difference between msg.value and maxAmountIn like below:

if(msg.value < maxAmountIn){
uint amountToPull = maxAmountIn - msg.value;
pullToken(vaultAsset, amountToPull, address(this));
}

Duplicate of #1

@github-actions github-actions bot added High A valid High severity issue Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label labels Sep 11, 2023
@sherlock-admin2 sherlock-admin2 changed the title Joyous Plastic Mallard - User can lost funds when mint() with ETH from LMPVaultRouter harisnabeel - User can lost funds when mint() with ETH from LMPVaultRouter Oct 3, 2023
@sherlock-admin2 sherlock-admin2 added the Reward A payout will be made for this issue label Oct 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label High A valid High severity issue Reward A payout will be made for this issue
Projects
None yet
Development

No branches or pull requests

2 participants