Core Contracts

Summary

liquidateBorrower function in StabilityPool contract is defined as follows:

It first calls _update internal function which mints scheduled RAAC rewards for RToken depositors. After that, it retrieves the user debt using lendingPool.getUserDebt function, which returns the user debt in underlying asset units:

We can see that the scaled debt balance is multiplied by the usage index to get the debt in underlying asset units. The problem is that liquidateBorrower function will apply a second multiplication scaling:

This leads to an excess debt value computation.

In parallel, both scaling ultimately use reserve.usageIndex(first directly, second through lending.Pool.getNormalizedDebt which is wrongly implemented (see other issue) and returns reserve.usageIndex(i.e., non-normalised index, corresponding to the last update of the reserve state).

This is problematic because before everything, lendingPool.updateState()should be called, in order to update the usage index of the reserve and be able to correctly compute the total debt to repay.

Overall, we can summarise:

• The more the index grows, the more the scaledUserDebt is over-estimated, because of the double multiplication scaling with the usage index

• The more time has passed since last reserve update, the more the user debt is under-estimated. Indeed, because scaledUserDebt is computed before updating the reserve state of the lending pool, the usage index used when computing the debt is lower than during finalizeLiquidation

Depending of the balance between these 2 factors, 2 situations are possible:

• In the case where the debt amount is over-estimated, which is likely to happen almost all the time after a certain usage index has been reached, this computation issue is not dramatic but still has consequences:

  • Impossibility to liquidate borrower even though the stability pool has enough crvUSD because of the following line: if (crvUSDBalance < scaledUserDebt) revert InsufficientBalance();. Because scaledUserDebt is over over-estimated, crvUSD balance could be enough to liquidate but not enough to pass that check.

  • If crvUSD balance is enough, liquidation can still proceed. Indeed, lendingPool.finalizationLiquidation is called, without specifying an amount

• In the case where the debt amount is under-estimated, which might happen especially if a long time has passed since last update or when the protocol launched recently with a low usage index, liquidation by the stability pool becomes impossible. Indeed, the stability pool doesn't approve enough token to be transferred with the line bool approveSuccess = crvUSDToken.approve(address(lendingPool), scaledUserDebt);. This means the following line in finalizeLiquidationin the lending pool will always fail : IERC20(reserve.reserveAssetAddress).safeTransferFrom(msg.sender, reserve.reserveRTokenAddress, amountScaled);

Impact

The impact of this issue is high as it can lead to impossibility to liquidate positions from the stability pool in some situations:

• Debt amount over-estimated and crvUSD balance of the stability pool just enough to liquidate the position but not enough to pass the check if (crvUSDBalance < scaledUserDebt) revert InsufficientBalance();

• Debt amount under-estimated with systematic revert because of insufficient approval amount by the stability pool

Tools Used

Manual review

Recommendations

1. Call lendingPool.updateState();before retrieving the user debt with getUserDebt(as it doesn't return the normalised user debt)

2. Remove excess multiplication scaling by not multiplying a second time by the index.