Core Contracts
Regnum Aurum Acquisition Corp
HardhatReal World AssetsNFT
77,280 USDC
View results
Previous Next
Submission Details
Severity: medium
Valid

Flaw in `StabilityPool.liquidateBorrower()` Leading to Late Reverts Due to Preliminarily Diminished Scaled Debt Calculation

0xmystery

Summary

The issue stems from an incorrect calculation of scaledUserDebt in StabilityPool.liquidateBorrower(), which may lead to unnecessary reverts at a late stage in Lending.finalizeLiquidation().

(Note: There's also a double scaling issue on userDebt in the code logic that I have reported separately.)

Vulnerability Details

Incorrect Calculation of scaledUserDebt in StabilityPool.liquidateBorrower()

As noted in the code logic below, getNormalizedDebt() is being referenced without updating the reserve state first, highly likely leading to an outdated or lower scaledUserDebt value.

StabilityPool.sol#L451-L453

// Get the user's debt from the LendingPool.
uint256 userDebt = lendingPool.getUserDebt(userAddress);
uint256 scaledUserDebt = WadRayMath.rayMul(userDebt, lendingPool.getNormalizedDebt());

If scaledUserDebt is underestimated, it falsely passes the following check:

StabilityPool.sol#L457-L458

uint256 crvUSDBalance = crvUSDToken.balanceOf(address(this));
if (crvUSDBalance < scaledUserDebt) revert InsufficientBalance();

The thing is crvUSDToken.balanceOf(address(this)) is technically the exact amount sent into the contract by the manager or the owner after querying the user's debt by incorporating the outdated reserve's normalized debt too via lendingPool.getNormalizedDebt():

LendingPool.sol#L597-L603

/**
* @notice Gets the reserve's normalized debt
* @return The normalized debt (usage index)
*/
function getNormalizedDebt() external view returns (uint256) {
return reserve.usageIndex;
}

This means the Stability Pool may unnecessarily proceed with finalizing a liquidation attempt that's bound to fail unless lendingPool.updateState() has atomically been called/utilized first both by the function logic as well as the caller.

Late Revert in lendingPool.finalizeLiquidation()

Later, in lendingPool.finalizeLiquidation(), the correctly scaled up debt amount is recalculated after having the state updated:

LendingPool.sol#L499-L525

// update state
ReserveLibrary.updateReserveState(reserve, rateData);
​
if (block.timestamp <= liquidationStartTime[userAddress] + liquidationGracePeriod) {
revert GracePeriodNotExpired();
}
​
UserData storage user = userData[userAddress];
​
uint256 userDebt = user.scaledDebtBalance.rayMul(reserve.usageIndex);

This userDebt (equal to amountScaled) is used in:

// Transfer reserve assets from Stability Pool to cover the debt
IERC20(reserve.reserveAssetAddress).safeTransferFrom(msg.sender, reserve.reserveRTokenAddress, amountScaled);

If scaledUserDebt was underestimated earlier, the actual required amountScaled might be greater than crvUSDToken.balanceOf() associated with StabilityPool.sol.

Impact

The transfer from the Stability Pool may revert and fail late in the execution flow, causing an unnecessary revert after multiple state updates have already occurred.

Additionally, if some liquidations fail due to outdated state, it may leave the system with unclaimed bad debt. This is especially true and exacerbated by the very fact that user.scaledDebtBalance is increasingly growing with exponential interests entailed. The longer the wait and delay, the more crvUSDToken will be needed.

Tools Used

Manual

Recommendations

Consider implementing the following refactoring. Simply update lending pool state BEFORE fetching user debt. No need to update state again before liquidation since finalizeLiquidation will do so:

StabilityPool.sol#L449-L470

function liquidateBorrower(address userAddress) external onlyManagerOrOwner nonReentrant whenNotPaused {
_update();
​
+ // Update lending pool state before getting the user's debt from the LendingPool.
+ lendingPool.updateState();
​
// Get the user's debt from the LendingPool.
uint256 userDebt = lendingPool.getUserDebt(userAddress);
uint256 scaledUserDebt = WadRayMath.rayMul(userDebt, lendingPool.getNormalizedDebt());
​
if (userDebt == 0) revert InvalidAmount();
​
uint256 crvUSDBalance = crvUSDToken.balanceOf(address(this));
if (crvUSDBalance < scaledUserDebt) revert InsufficientBalance();
​
// Approve the LendingPool to transfer the debt amount
bool approveSuccess = crvUSDToken.approve(address(lendingPool), scaledUserDebt);
if (!approveSuccess) revert ApprovalFailed();
- // Update lending pool state before liquidation
- lendingPool.updateState();
​
// Call finalizeLiquidation on LendingPool
lendingPool.finalizeLiquidation(userAddress);
​
emit BorrowerLiquidated(userAddress, scaledUserDebt);
}
Updates

Lead Judging Commences

inallhonesty Lead Judge 4 months ago
Submission Judgement Published
Validated
Assigned finding tags:

StabilityPool: liquidateBorrower should call lendingPool.updateState earlier, to ensure the updated usageIndex is used in calculating the scaledUserDebt

Support

FAQs

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