code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-07-moonwell/blob/fced18035107a345c31c9a9497d0da09105df4df/src/core/Comptroller.sol#L689-L698

Vulnerability details

Impact

The _setCloseFactor is a function used to set the closeFactorMantissa, however there are no boundaries in this function. An admin error such as missing one zero in the new closeFactor can cause multiple accounts to be liquidatable.

Proof of Concept

This issue affects the function liquidateBorrowAllowed, whereby if the closefactor is too small,the function returns zero

        /* The borrower must have shortfall in order to be liquidatable */
        (Error err, , uint shortfall) = getAccountLiquidityInternal(borrower);
        if (err != Error.NO_ERROR) {
            return uint(err);
        }
        if (shortfall == 0) {
            return uint(Error.INSUFFICIENT_SHORTFALL);
        }

        /* The liquidator may not repay more than what is allowed by the closeFactor */
        uint borrowBalance = MToken(mTokenBorrowed).borrowBalanceStored(borrower);
        uint maxClose = mul_ScalarTruncate(Exp({mantissa: closeFactorMantissa}), borrowBalance);
        if (repayAmount > maxClose) {
            return uint(Error.TOO_MUCH_REPAY);
        }

        return uint(Error.NO_ERROR);
    }

This harms users in the protocol considerably due to an admin error.

Tools Used

Manual Review

Recommended Mitigation Steps

Set a reasonable minimum value for the closeFactor and require the new value is greater than the min value before changing.

Assessed type

Other

c4-pre-sort commented 1 year ago

0xSorryNotSorry marked the issue as duplicate of #187

c4-judge commented 1 year ago

alcueca changed the severity to QA (Quality Assurance)