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

1 stars 0 forks source link

Borrow interest rate model open for higher rate than it designed #319

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

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

Vulnerability details

Impact

Borrow interest rate model open for higher rate than it designed

Proof of Concept

In Moonwell protocol (and Compound v2), the accrueInterest() is the common function which calculates interest accrued from the last checkpointed block up to the current block and writes new checkpoint to storage.

There are lines which fetch the interest rate (borrowRateMantissa) as shown below:

File: MToken.sol
401:         /* Calculate the current borrow interest rate */
402:         uint borrowRateMantissa = interestRateModel.getBorrowRate(cashPrior, borrowsPrior, reservesPrior);
403:         require(borrowRateMantissa <= borrowRateMaxMantissa, "borrow rate is absurdly high");

This borrowRateMantissa is fetched from interestRateModel.getBorrowRate, but the issue here is the check of borrowRateMaxMantissa. This Line 403, require(borrowRateMantissa <= borrowRateMaxMantissa, "borrow rate is absurdly high"); is to make sure the borrow rate fall under the configured borrowRateMaxMantissa.

File: MTokenInterfaces.sol
22:     /// @notice Maximum borrow rate that can ever be applied (.0005% / block)
23:     uint internal constant borrowRateMaxMantissa = 0.0005e16;

Compound v2, which is the parent fork of Moonwell uses blocks for calculations and Moonwell uses seconds (timestamp), baseRatePerBlock vs baseRatePerTimestamp.

Most of Moonwell configuration are mimicing Compound v2, for example the above borrowRateMaxMantissa uses the same value for constant borrowRateMaxMantissa in Compound.

what we need to focus here, in the comment it says that borrow rate value is applied per block,

Maximum borrow rate that can ever be applied (.0005% / block)

meanwhile the Moonwell use rate per timestamp rather than block.

Assuming block time on L1 where Compound v2 is deployed is 12 seconds, we can safely assume, the current borrowRateMaxMantissa value should be 12 times lower.


For a reference, a similar issue found on Sonne protocol, which is audited by yAudit (team from yearn finance) for a clearer explanation https://reports.yaudit.dev/reports/05-2023-Sonne/#3-medium---sonne-interest-rate-model-math-error

Another Compound fork on Optimism, Hundred Finance, uses the correct value 0.00004e16.

Note: the borrowRateMaxMantissa variable exist on MTokenInterfaces which is not in scope for audit, but where this borrowRateMaxMantissa is being used is in MToken, which is in scope. We can focus the issue on the MToken contract just to make sure that this finding will not be rejected because of the 'scope' reasoning.

Tools Used

Manual analysis

Recommended Mitigation Steps

Change borrowRateMaxMantissa value to be 12 times lower.

Assessed type

Math

c4-pre-sort commented 1 year ago

0xSorryNotSorry marked the issue as duplicate of #18

c4-judge commented 1 year ago

alcueca marked the issue as satisfactory