Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
code423n4 opened this issue Jul 31, 2023 · 6 comments
Closed
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working downgraded by judge Judge downgraded the risk level of this issue duplicate-92 low quality report This report is of especially low quality sponsor disputed Sponsor cannot duplicate the issue, or otherwise disagrees this is an issue unsatisfactory does not satisfy C4 submission criteria; not eligible for awards

Comments

@code423n4
Copy link
Contributor

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
code-423n4/2023-03-asymmetry-findings#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

@code423n4 code423n4 added 3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working labels Jul 31, 2023
code423n4 added a commit that referenced this issue Jul 31, 2023
@0xSorryNotSorry
Copy link

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
Copy link

0xSorryNotSorry marked the issue as low quality report

@c4-pre-sort c4-pre-sort added the low quality report This report is of especially low quality label Aug 1, 2023
@c4-sponsor
Copy link

ElliotFriedman marked the issue as sponsor disputed

@c4-sponsor c4-sponsor added the sponsor disputed Sponsor cannot duplicate the issue, or otherwise disagrees this is an issue label Aug 3, 2023
@c4-judge
Copy link
Contributor

alcueca changed the severity to 2 (Med Risk)

@c4-judge c4-judge added 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value downgraded by judge Judge downgraded the risk level of this issue and removed 3 (High Risk) Assets can be stolen/lost/compromised directly labels Aug 15, 2023
@c4-judge
Copy link
Contributor

alcueca marked the issue as duplicate of #92

@c4-judge
Copy link
Contributor

alcueca marked the issue as unsatisfactory:
Out of scope

@c4-judge c4-judge added the unsatisfactory does not satisfy C4 submission criteria; not eligible for awards label Aug 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working downgraded by judge Judge downgraded the risk level of this issue duplicate-92 low quality report This report is of especially low quality sponsor disputed Sponsor cannot duplicate the issue, or otherwise disagrees this is an issue unsatisfactory does not satisfy C4 submission criteria; not eligible for awards
Projects
None yet
Development

No branches or pull requests

5 participants