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

TangYuanShen - LMPVaultRouter will charge a user twice and the extra funds can be stolen #825

Closed
sherlock-admin2 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-admin2
Copy link
Contributor

sherlock-admin2 commented Aug 30, 2023

TangYuanShen

high

LMPVaultRouter will charge a user twice and the extra funds can be stolen

Summary

The LMPVaultRouter will charge a user twice for a deposit or mint to an LMPVault, if the user wants to use ETH. This extra ETH can then be stolen by an attacker.

Vulnerability Detail

The LMPVaultRouter inherits from LMPVaultRouterBase, which exposes the functions deposit and mint where a user can deposit assets to an LMPVault.

Both functions use _processEthIn which will wrap msg.value to WETH, in case the user wants to use native ETH:

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

But after this call, the functions will still try to pull the tokens from the user (which would be WETH):

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

As a result, if the user sends ETH, they will be charged twice, once in ETH, once in WETH. Only the specific amount will be used and so extra WETH will stay in the contract.

The contract also inherits from PaymentPeriphery, which would allow an attacker to immediately pull the extra WETH from the contract after such as a mistake, using sweepToken:

function sweepToken(IERC20 token, uint256 amountMinimum, address recipient) public payable {
    uint256 balanceToken = token.balanceOf(address(this));
    if (balanceToken < amountMinimum) revert InsufficientToken();

    if (balanceToken > 0) {
        token.safeTransfer(recipient, balanceToken);
    }
}

Impact

The LMPVaultRouter will charge a user twice if ETH is used and the extra amount of WETH can be immediately stolen by an attacker, resulting in a loss of user 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, VSCode

Recommendation

The case of sending ETH should be correctly handled in deposit and mint by checking the balance of WETH or using some return value of _processEthIn.

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-admin sherlock-admin changed the title Oblong Aqua Grasshopper - LMPVaultRouter will charge a user twice and the extra funds can be stolen TangYuanShen - LMPVaultRouter will charge a user twice and the extra funds can be stolen Oct 3, 2023
@sherlock-admin sherlock-admin 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