Moonwell
Moonwell
DeFiFoundry
15,000 USDC
View results
Previous Next
Submission Details
Severity: medium
Invalid

Users without shortfalls will get wrongfully liquidated.

ljj

Summary

Users will get wrongfully liquidated if they have paid their badDebt and no longer have a shortfall.

Vulnerability Details

In the proposed update, users with badDebt are documented in mFRAX.json and mxcDOT.json . mip-m17.sol will loop through all these users with shortfalls and call fixUser() for the users with shortfalls , for example:

string memory debtorsRaw = string(
abi.encodePacked(
vm.readFile("./src/proposals/mips/mip-m17/mFRAX.json")
)
);
bytes memory debtorsParsed = vm.parseJson(debtorsRaw);
Accounts[] memory mFRAXDebtors = abi.decode(
debtorsParsed,
(Accounts[])
);
address mFRAXAddress = addresses.getAddress("MOONWELL_mFRAX");
IMErc20Delegator mFRAXDelegator = IMErc20Delegator(mFRAXAddress);
for (uint256 i = 0; i < mFRAXDebtors.length; i++) {
if (
mFRAXDelegator.borrowBalanceStored(mFRAXDebtors[i].addr) > 0
) {
_pushAction(
mFRAXAddress,
abi.encodeWithSignature(
"fixUser(address,address)",
reallocationMultisig,
mFRAXDebtors[i].addr
),
string(
abi.encodePacked(
"Liquidate bad mFRAX debt for user: ",
Strings.toHexString(mFRAXDebtors[i].addr)
)
)
);
}

And we can see that in mip-m17.sol and fixUsers() there are no more checks done to see if the user in the .json file still has a shortfall, only checks that are done are to see if the user has any borrowed amount.

In a scenario where a user in the .json file pay off a part of their bad debt to get to a healthy position and remove their shortfall, while still having some borrowBalance this user will be wrongfully liquidated.

Impact

Wrongful liquidation of a user without shortfall, causing loss of funds for the user. This is a low likelihood but high impact vulnerability.

Tools Used

Manual review.

Recommendations

liquidateBorrowAllowed() function in Comptroller.sol checks if a users liquidation should be allowed to occur. A call to this function can be implemented in fixUsers() and transaction should revert if the liquidation of this user shouldn’t occur. In liquidateBorrowAllowed() a check is done to see if the user has a shortfall.

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

Alternatively implement the checks above in fixUser() . It is possible to remove currently implemented borrowBalance checks with these fixes to save gas.

Updates

Lead Judging Commences

0xnevi Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Other

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.