Core Contracts

Summary

liquidateBorrower calculates userDebt during the liqudation, to know how much debt should be covered. But there is a mistake in calculation, because scaledDebtBalance is multiplied twice by userIndex, while it should happened once.

scaledDebtBalance is a field that is assigned in the borrow function. It allows to track changes in userIndex from the time of borrowing to the time of liquidation.

It works in following way:

• user borrows 100 usd, at index 1.1, so borrow amount is divied by index ( his scaledDebtBalance is equal to 90.9090909090909 )

• user wants to pay it back ( or is liquidated ) at index 1.2. so now his borrow is multiply by index. And we know, he needs to pay back 109.09090909090908 CRVusd

Vulnerability Details

userDebt is fetched with getUserDebt function and later it is multiply by getNormalizedDebt, but the problem here is that getUserDebt already returns scaledDebtBalance multiplied by usageIndex.

Impact

scaledUserDebt will be bigger than it should be.

This value is used in 3 places:

• line 7: checks if contract has enough tokens for paying debts, so there might be a case, when it has enough tokens but less than scaledUserDebt ( which is bigger than it should be ), in such case it will revert.

• line 10: it approves more tokens than it should

• line 18: Wrong event is emitted, lendingPool.finalizeLiquidation will correctly recalculate debts to pay, so this event is emitted with wrong value.

Important thing is that bug covers another bug

lendingPool.updateState(); is called after userDebt is calculated.

updateState is actually updating usageIndex, so this debt is calculated with old indexes. So it has impact on all points listed above, but the most importantly it has impact on tokens approval (line 10).

• stabilityPool approves some amount calculated base on old usageIndexes

• usage indexses are updated in line 13 ( it can only go up, and in most case it will change at least slightly )

• lendingPool tries to finalize liquidation, but it can't, since it will recalculate the debt, and it will be slightly bigger than approved amounts of tokens, so the transaction will be reverted.

That second case would make liquidating the user a lot harder. This bug is hidden, because scaledUserDebt would be always bigger than it should be. So the second bug might happen only under the assumtion, that the first one would be fixed.

Tools Used

Manual Review

Recommendations

scaledDebtBalance should be multiplied by usageIndex only once, and move lendingPool.updateState(); next to _update();