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 during deposit() in LMPVaultRouter #728

Closed
sherlock-admin opened this issue Aug 30, 2023 · 0 comments
Closed
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 during deposit() in LMPVaultRouter

Summary

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

Vulnerability Detail

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

deposit(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, amount, address(this));

3 - The user will get shares of 1 ETH, and user 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#L51
https://github.com/sherlock-audit/2023-06-tokemak/blob/main/v2-core-audit-2023-07-14/src/vault/LMPVaultRouterBase.sol#L54

Impact

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

Code Snippet

   /// @inheritdoc ILMPVaultRouterBase
    function deposit(
        ILMPVault vault,
        address to,
        uint256 amount,
        uint256 minSharesOut
    ) public payable virtual override returns (uint256 sharesOut) {
        // handle possible eth
        _processEthIn(vault); // @audit

        IERC20 vaultAsset = IERC20(vault.asset());
        pullToken(vaultAsset, amount, address(this)); // @audit

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

Tool used

Manual Review

Recommendation

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

if(msg.value < amount){
uint amountToPull = amount - 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 during deposit() in LMPVaultRouter harisnabeel - User can lost funds during deposit() in 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