code423n4 commented 11 months ago

Lines of code

https://github.com/code-423n4/2023-07-moonwell/blob/main/src/core/Comptroller.sol#L394-L424

Vulnerability details

Impact

The Mtoken markets configured for the respective collateral asset types can get deprecated due to various reasons associated with those assets. There should be functionality in the Comptroller.liquidateBorrowAllowed function to liquidate all the borrows in the accounts for deprecated markets.

This functionality is present in the Comptroller.liquidateBorrowAllowed of the compound protocol, but is missing in the Comptroller.liquidateBorrowAllowed of the moonwell protocol.

Since this isDeprecated functionality is missing the borrowed assets of the deprecated MToken will not able to be repaid after the deprecation.

Following code snippet shows how the isDeprecated functionality is implemented in Compound.Comptroller.sol.

function isDeprecated(CToken cToken) public view returns (bool) {
    return
        markets[address(cToken)].collateralFactorMantissa == 0 &&
        borrowGuardianPaused[address(cToken)] == true &&
        cToken.reserveFactorMantissa() == 1e18
    ;
}

Proof of Concept

    function liquidateBorrowAllowed(
        address mTokenBorrowed,
        address mTokenCollateral,
        address liquidator,
        address borrower,
        uint repayAmount) override external view returns (uint) {
        // Shh - currently unused
        liquidator;

        if (!markets[mTokenBorrowed].isListed || !markets[mTokenCollateral].isListed) {
            return uint(Error.MARKET_NOT_LISTED);
        }

        /* 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);
    }

https://github.com/code-423n4/2023-07-moonwell/blob/main/src/core/Comptroller.sol#L394-L424

Tools Used

Manual Review and VSCode

Recommended Mitigation Steps

Hence it is recommended to add this functionality and set the deprecated flag to true for the tokens which are going to be deprecated thus allowing the borrowed positions of the deprecated token to be immediately liquidated thus keeping the protocol stable and safe. The isDeprecated should be called inside the Comptroller.liquidateBorrowAllowed function to enable liquidation.

Assessed type

Other

c4-pre-sort commented 11 months ago

0xSorryNotSorry marked the issue as primary issue

c4-sponsor commented 11 months ago

ElliotFriedman marked the issue as sponsor acknowledged

c4-judge commented 11 months ago

alcueca marked the issue as satisfactory

c4-judge commented 10 months ago

alcueca marked the issue as selected for report