bin2chen - deposit() may overflow

[sherlock-admin2]

bin2chen

medium

deposit() may overflow

Summary

maxDeposit() use formula : convertToAssets(_maxMint(wallet))
due to _maxMint()may be equal to type(uint256).max
lead to convertToAssets(type(uint256).max) overflow
lead todeposit() forever revert

Vulnerability Detail

Currently maxDeposit() uses the formula: maxAssets = convertToAssets(_maxMint(wallet));

It will overflow when the following two conditions are met

1. totalSupplyLimit and perWalletLimit are not limited by default. i.e.: totalSupplyLimit = type(uint256).max) perWalletLimit = type(uint256).max).

2. LMPVault.sol is profitable, i.e.: totalAssets > totalSupply

We simplify the above equation to: maxAssets = maxMint.mulDiv(totalAssets, totalSupply, Math.Rounding.Up)

The following code demonstrates that in this case maxDeposit() will revert

add to LMPVault-Withdraw.t.sol

    using Math for uint256;
    function test_MaxDeposit() public{
        uint256 maxMint = type(uint256).max; // if not limt totalSupply and perWalletLimit
        uint256 totalSupply = 1000;
        uint256 totalAssets = 1001;  //profit
        maxMint.mulDiv(totalAssets, totalSupply, Math.Rounding.Up);
    }

 forge test --match-test test_MaxDeposit -vvv 

Running 1 test for test/vault/LMPVault-Withdraw.t.sol:LMPVaultMintingTests
[FAIL. Reason: EvmError: Revert] test_MaxDeposit() (gas: 464)
Traces:
  [464] testContract::test_MaxDeposit() 
    └─ ← "EvmError: Revert"

Test result: FAILED. 0 passed; 1 failed; 0 skipped; finished in 10.91ms
Ran 1 test suites: 0 tests passed, 1 failed, 0 skipped (1 total tests)

Failing tests:
Encountered 1 failing test in test/vault/LMPVault-Withdraw.t.sol:LMPVaultMintingTests
[FAIL. Reason: EvmError: Revert] test_MaxDeposit() (gas: 464)

Encountered a total of 1 failing tests, 0 tests succeeded

In the current protocol, by default there is no limit on totalSupplyLimit/perWalletLimit (test cases are also use Max)
So when there is profitability, LMPVault.deposit() fails and the execution will always revert

Impact

When there is profit, LMPVault.deposit() fails and execution will always revert

Code Snippet

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

Tool used

Manual Review

Recommendation

    function maxDeposit(address wallet) public view virtual override returns (uint256 maxAssets) {
-        maxAssets = convertToAssets(_maxMint(wallet));
+        uint256 maxMint = _maxMint(wallet);
+        if (maxMint == type(uint256).max) return type(uint256).max;
+        maxAssets = convertToAssets(maxMint);
    }

Duplicate of #577

[sherlock-admin2]

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

Trumpero commented:

low, totalSupplyLimit and perWalletLimit will be set by the admin (in the constructor and setter function). The deposit() function is disabled until these parameters are set, and it does not cause any loss