code-423n4 / 2023-07-moonwell-findings

1 stars 0 forks source link

First mint user can inflate share which can steal asset from other user #375

Closed code423n4 closed 10 months ago

code423n4 commented 11 months ago

Lines of code

https://github.com/code-423n4/2023-07-moonwell/blob/main/src/core/MToken.sol#L340-L370

Vulnerability details

Impact

A well know inflation attack/first deposit (mint) bug. The attacker can steal assets from other user's deposit (mint).

Proof of Concept

The Moonwell project is a fork from the Compound Protocol. The MToken (the MToken on Compound) represents a yield-bearing asset that is generated when a user deposits underlying tokens. The number of MTokens minted for a user depends on the amount of underlying tokens they are depositing.

The MToken contract calculates the amount of MTokens to be minted in two scenarios:

  1. First deposit: This occurs when MToken.totalSupply() is 0, indicating that there are no existing MTokens in circulation.
  2. All subsequent deposits: After the initial deposit, any additional deposits made by the user fall under this case.

Below is the relevant portion of the MToken code

File: MToken.sol
335:     /**
336:      * @notice Calculates the exchange rate from the underlying to the MToken
337:      * @dev This function does not accrue interest before calculating the exchange rate
338:      * @return (error code, calculated exchange rate scaled by 1e18)
339:      */
340:     function exchangeRateStoredInternal() virtual internal view returns (MathError, uint) {
341:         uint _totalSupply = totalSupply;
342:         if (_totalSupply == 0) {
343:             /*
344:              * If there are no tokens minted:
345:              *  exchangeRate = initialExchangeRate
346:              */
347:             return (MathError.NO_ERROR, initialExchangeRateMantissa);
348:         } else {
349:             /*
350:              * Otherwise:
351:              *  exchangeRate = (totalCash + totalBorrows - totalReserves) / totalSupply
352:              */
353:             uint totalCash = getCashPrior();
354:             uint cashPlusBorrowsMinusReserves;
355:             Exp memory exchangeRate;
356:             MathError mathErr;
357:
358:             (mathErr, cashPlusBorrowsMinusReserves) = addThenSubUInt(totalCash, totalBorrows, totalReserves);
359:             if (mathErr != MathError.NO_ERROR) {
360:                 return (mathErr, 0);
361:             }
362:
363:             (mathErr, exchangeRate) = getExp(cashPlusBorrowsMinusReserves, _totalSupply);
364:             if (mathErr != MathError.NO_ERROR) {
365:                 return (mathErr, 0);
366:             }
367:
368:             return (MathError.NO_ERROR, exchangeRate.mantissa);
369:         }
370:     }

The function contains a critical vulnerability that can be exploited to steal funds from initial depositors of a newly deployed MToken contract.

The vulnerability arises from the fact that the exchange rate of the MToken is determined by the ratio of the MToken's totalSupply and the underlying token balance of the MToken contract. An attacker can take advantage of this to manipulate the exchange rate.

The attack as follow: After the MToken has been deployed and added to the lending protocol, the attacker mints the smallest possible amount of MTokens. The attacker then performs a straightforward transfer of underlying tokens to the MToken contract, artificially inflating the value of underlying.balanceOf(MToken). Then when a legitimate user makes their deposit, the mintTokens value for the user will be reduced to less than 1 and rounded down to 0. Consequently, the user will receive 0 MTokens against their deposit, and the entire supply of MTokens will be held by the attacker.

As a result, the attacker can redeem their MToken balance for the entire underlying token balance of the MToken contract. This process can be repeated to steal subsequent users' deposits.

Some reference of this issues: https://mixbytes.io/blog/overview-of-the-inflation-attack https://code4rena.com/reports/2023-01-ondo#m-02-first-deposit-bug https://github.com/code-423n4/2023-03-asymmetry-findings/issues/715

Tools Used

Manual analysis

Recommended Mitigation Steps

Need to enforce a minimum deposit that can't be withdrawn, for instance mint some of the intial amount to the zero address

Assessed type

Token-Transfer

0xSorryNotSorry commented 11 months ago

Then when a legitimate user makes their deposit, the mintTokens value for the user will be reduced to less than 1 and rounded down to 0. Consequently, the user will receive 0 MTokens against their deposit, and the entire supply of MTokens will be held by the attacker.

Above statement requires further proof which was not provided in this submission. Marking as LQ.

c4-pre-sort commented 11 months ago

0xSorryNotSorry marked the issue as low quality report

c4-sponsor commented 11 months ago

ElliotFriedman marked the issue as sponsor disputed

c4-judge commented 10 months ago

alcueca changed the severity to 2 (Med Risk)

c4-judge commented 10 months ago

alcueca marked the issue as duplicate of #92

c4-judge commented 10 months ago

alcueca marked the issue as unsatisfactory: Out of scope