Vulnerability Details
when repaying bad debt with reserves it compares the totalReserves and badDebt to determine which value to use/sufficient to repay the debt.
https://github.com/Cyfrin/2024-03-Moonwell/blob/main/src/MErc20DelegateFixer.sol#L49
function repayBadDebtWithReserves() external nonReentrant {
@> uint256 currentReserves = totalReserves;
@> uint256 currentBadDebt = badDebt;
require(currentReserves != 0, "reserves are zero");
require(currentBadDebt != 0, "bad debt is zero");
@> uint256 subtractAmount = currentBadDebt < currentReserves
? currentBadDebt
: currentReserves;
@> badDebt = SafeMath.sub(currentBadDebt, subtractAmount);
totalReserves = SafeMath.sub(currentReserves, subtractAmount);
...SNIP
}
but totalReserves is out of sync with it actual value since the interest accrued upto that point is not updated.
Within accrueInterest():
function accrueInterest() public returns (uint) {
...SNIP//TRUNCATED for brevity
accrualBlockTimestamp = currentBlockTimestamp;
borrowIndex = borrowIndexNew;
totalBorrows = totalBorrowsNew;
@> totalReserves = totalReservesNew;
emit AccrueInterest(cashPrior, interestAccumulated, borrowIndexNew, totalBorrowsNew);
return uint(Error.NO_ERROR);
}
Impact
totalReserves would be out of sync with it's actual value, the actual value might be smaller or larger that the current reserves at that point.
Tools Used
Manual Review
Recommendations
call accrueInterest() before utilizing the totalReserves storage
function repayBadDebtWithReserves() external nonReentrant {
+ accrueInterest()
uint256 currentReserves = totalReserves;
uint256 currentBadDebt = badDebt;
require(currentReserves != 0, "reserves are zero");
require(currentBadDebt != 0, "bad debt is zero");
/// no reverts possible past this point
/// take the lesser of the two, subtract it from both numbers
uint256 subtractAmount = currentBadDebt < currentReserves
? currentBadDebt
: currentReserves;
/// bad debt -= subtract amount
badDebt = SafeMath.sub(currentBadDebt, subtractAmount);
/// current reserves -= subtract amount
totalReserves = SafeMath.sub(currentReserves, subtractAmount);
emit BadDebtRepayedWithReserves(
badDebt,
currentBadDebt,
totalReserves,
currentReserves
);
}
