Moonwell

Summary

When liquidating users in proposal contract - this is a code excerpt used to perform the action:

This is the condition that must pass in order for the user to be liquidated:

Vulnerability Details

The if condition used to verify that user is eligible to be liquidated is not reading correct state because it does not take into account most recent interest accrual the way it does in MErc20DelegateFixer.sol in fixUser(address, address).

Proposal is not using correct contract state in comparison when performing liquidation.

Call to accrueInterest() may fail as is seen in contract logic. It can fail for many reasons. As seen in function definition:
https://github.com/Cyfrin/2024-03-Moonwell/blob/e57b8551a92824d35d4490f5e7f27c373be172bd/src/MToken.sol#L384

Proposal contract must ensure its safe execution by calling accrueInterest() before pushing liquidation actions on new implementation contract.

We put an emphasis on this action because it is the only action that may cause a failure when liquidating a user that is not impacted by function arguments. Other conditions that must pass are dependent on provided function arguments such as user , liquidator and msg.sender. As seen here:
https://github.com/Cyfrin/2024-03-Moonwell/blob/e57b8551a92824d35d4490f5e7f27c373be172bd/src/MErc20DelegateFixer.sol#L83-L95

Impact

Proposal will fail - and the protocol is left with an unusable function which could have been avoided by placing an require(accrueInterest() == 0, "") check on the proposal contract.

Tools Used

Manual review

Recommendations

Add a line to mip-m17.sol contract before any call to fixUser(address, address) that requires a call to accrueInterest() function to pass without any errors and to return 0.