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

lil.eth - First depositor on LMPVault.sol can break minting of shares #112

Closed
sherlock-admin opened this issue Aug 29, 2023 · 1 comment
Closed
Labels
Excluded Excluded by the judge without consulting the protocol or the senior Non-Reward This issue will not receive a payout

Comments

@sherlock-admin
Copy link
Contributor

sherlock-admin commented Aug 29, 2023

lil.eth

high

First depositor on LMPVault.sol can break minting of shares

Summary

The common "first depositor" vulnerability is found in LMPVault#deposit(). The first account to deposit into the LMPVault can steal value from subsequent depositors by:

  • Minting 1 wei shares
  • Directly transferring assets into the contract to inflate the totalAssets value
  • Subsequent depositors deposit assets but are minted 0 shares due to precision loss (+ Math.rounding.down specification)
  • First depositor steals the assets

Vulnerability Detail

The depositor's shares are calculated via:

//1
    function deposit( 
        uint256 assets,
        address receiver
    ) public virtual override nonReentrant noNavChange ensureNoNavOps returns (uint256 shares) {
       ....
        shares = previewDeposit(assets);
       ... //E other logic to mint shares
    }
//2 
    function previewDeposit(uint256 assets) public view virtual returns (uint256 shares) {
        shares = _convertToShares(assets, Math.Rounding.Down);
    }
//3 
    //E assets to shares with rounding direction
    function _convertToShares(uint256 assets, Math.Rounding rounding) internal view virtual returns (uint256 shares) {
        uint256 supply = totalSupply();

        //E shares = (assets == 0 || totalSupply() == 0) ? assets : assets * totalSupply() / totalAssets() +- rounding
        shares = (assets == 0 || supply == 0) ? assets : assets.mulDiv(supply, totalAssets(), rounding);
    }

Upon first deposit, the totalAssets() value will be 0. The attacker will transact with an amount = 1 wei to mint 1 wei of shares. Then the attacker will transfer some value of asset directly to the contract. For this example, the attacker transfers 10,000 USDC.

Next, a subsequent depositor attempts to mint shares with 5,000 USDC.

shares = 5000 USDC * 1 wei / 10,000 USDC = 0 due to precision loss.

The attacker can now withdraw the second depositor's assets.

Other vulnerabilities related to this one can be found here : OpenZeppelin/openzeppelin-contracts#3706

Impact

Theft of deposit

Code Snippet

https://github.com/sherlock-audit/2023-06-tokemak/blob/main/v2-core-audit-2023-07-14/src/vault/LMPVault.sol#L332

Tool used

Manual Review

Recommendation

Mint a certain number of shares and transfer them to address(0) within the initialize() function.
Refer to this article for better understanding/remediation

@github-actions github-actions bot added the Excluded Excluded by the judge without consulting the protocol or the senior label Sep 11, 2023
@sherlock-admin2
Copy link
Contributor

1 comment(s) were left on this issue during the judging contest.

Trumpero commented:

invalid. totalAsset = totalIdle + totalDebt, totalAsset doesn't use balance of contract. Any deposit will increase totalIdle, so first depositor can't inflate shares of LMPVault

@sherlock-admin2 sherlock-admin2 changed the title Fantastic Grey Bird - First depositor on LMPVault.sol can break minting of shares lil.eth - First depositor on LMPVault.sol can break minting of shares Oct 3, 2023
@sherlock-admin2 sherlock-admin2 added the Non-Reward This issue will not receive a payout label Oct 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Excluded Excluded by the judge without consulting the protocol or the senior Non-Reward This issue will not receive a payout
Projects
None yet
Development

No branches or pull requests

2 participants