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

1 stars 0 forks source link

liquidateBorrow() mTokens that do not enter the market can still be liquidated as collateral #227

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

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

Vulnerability details

Impact

borrower's mTokens that do not join the market, but it still be Liquidation as collateral

Proof of Concept

If user wants to use mToken as collateral, the user needs to enter market with enterMarket() function.

/**
     * @notice Add assets to be included in account liquidity calculation
     * @param mTokens The list of addresses of the mToken markets to be enabled
     * @return Success indicator for whether each corresponding market was entered
     */
    function enterMarkets(address[] memory mTokens) override public returns (uint[] memory) {

Those who have joined the market can exit with the exitMarket() function.

/**
     * @notice Removes asset from sender's account liquidity calculation
     * @dev Sender must not have an outstanding borrow balance in the asset,
     *  or be providing necessary collateral for an outstanding borrow.
     * @param mTokenAddress The address of the asset to be removed
     * @return Whether or not the account successfully exited the market
     */
    function exitMarket(address mTokenAddress) override external returns (uint) {

So if it is not in the market, it should not be used as collateral, and this agreement should also use for Liquidation. But the current Liquidation method does not determine whether mTokenCollateral is in the market or not. The liquidator can seize mToken that has not entered a market.

function liquidateBorrowInternal(address borrower, uint repayAmount, MTokenInterface mTokenCollateral) internal nonReentrant returns (uint, uint) {

Liquidation functions only determine whether mToken is a legitimate market, and does not determine whether the borrower has added it to the market(markets[address(mToken)].accountMembership[borrower] == true)

So there should be a restriction that the liquidator needs to choose one of the collaterals that this borrower has joined the market, not one that has not joined the market

Tools Used

Manual Review

Recommended Mitigation Steps

liquidateBorrowAllowed() function needs to check accountMembership is true for mToken.

Assessed type

Context

c4-pre-sort commented 1 year ago

0xSorryNotSorry marked the issue as primary issue

ElliotFriedman commented 1 year ago

known issue, please read known issues as this is excluded from the contest

c4-sponsor commented 1 year ago

ElliotFriedman marked the issue as sponsor disputed

c4-judge commented 1 year ago

alcueca marked the issue as unsatisfactory: Invalid