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

Liquidation failure due to incorrect userDebt scaling in liquidateBorrower()

tigerfrake

Summary

The liquidateBorrower() function in the StabilityPool contract incorrectly scales the user's debt when attempting to liquidate a borrower. The function retrieves the user's debt using getUserDebt(), which already normalizes the debt based on the current usageIndex. However, the liquidateBorrower() function then applies an additional scaling operation, resulting in a double scaling of the debt amount.

This unnecessary operation leads to inaccuracies in the approval amount for token transfers.

Vulnerability Details

Here is how liquidateBorrower() works:

function liquidateBorrower(address userAddress) external onlyManagerOrOwner nonReentrant whenNotPaused {
---SNIP---
uint256 userDebt = lendingPool.getUserDebt(userAddress);
// @audit-issue Double scaling done here
>> uint256 scaledUserDebt = WadRayMath.rayMul(userDebt, lendingPool.getNormalizedDebt());
​
if (userDebt == 0) revert InvalidAmount();
​
uint256 crvUSDBalance = crvUSDToken.balanceOf(address(this));
if (crvUSDBalance < scaledUserDebt) revert InsufficientBalance();
​
// @audit-issue Approval made based on incorrect value
>> bool approveSuccess = crvUSDToken.approve(address(lendingPool), scaledUserDebt);
if (!approveSuccess) revert ApprovalFailed();
---SNIP---
}

This function retrieves userDebt by lendingPool.getUserDebt():

function getUserDebt(address userAddress) public view returns (uint256) {
UserData storage user = userData[userAddress];
>> return user.scaledDebtBalance.rayMul(reserve.usageIndex);
}

Notice that this function already normalizes the scaledDebtBalance by rayMul(reserve.usageIndex) meaning that the value returned here is already in underlying asset units.
However, the liquidateBorrower() proceeds to rescale this into scaledUserDebt: WadRayMath.rayMul(userDebt, lendingPool.getNormalizedDebt()) which does the same thing already done by getUserDebt() to an already normalized value.

Then, it proceeds to set approval using this final value.

However, lets see what value is actually pulled in finalizeLiquidation():

UserData storage user = userData[userAddress];
// @audit-info userDebt is scaled once here
>> uint256 userDebt = user.scaledDebtBalance.rayMul(reserve.usageIndex);
​
---SNIP---
​
// @audit-info that userDebt is passed in burn() as `amount`
>> (uint256 amountScaled, uint256 newTotalSupply, uint256 amountBurned, uint256 balanceIncrease) = IDebtToken(reserve.reserveDebtTokenAddress).burn(userAddress, userDebt, reserve.usageIndex);
​
// @audit-info returned amountScaled is pulled from the stability pool
>> IERC20(reserve.reserveAssetAddress).safeTransferFrom(msg.sender, reserve.reserveRTokenAddress, amountScaled);
​

Now lets see how DebtToken.burn() handles all this and the values it returns:

// @audit-info the userDebt passed as amount is scaled
uint256 amountScaled = amount.rayDiv(index);
​
if (amountScaled == 0) revert InvalidAmount();
// @audit-info will actually burn amountScaled
_burn(from, amount.toUint128());
emit Burn(from, amountScaled, index);
// @audit-info `amount` passed (userDebt) is returned as first element
>> return (amount, totalSupply(), amountScaled, balanceIncrease);

Notice that in finalizeLiquidation(), the first return value (amountScaled) is what the contract pulls from stability pool. And as we have seen, this is actually the same userDebt that was passed to burn().
Therefore, there is really a big mismatch between what was approved and what is being pulled.

Impact

The incorrect scaling of userDebt can result in the approval amount being set inaccurately, which may not reflect the actual amount needed to repay the user's debt. Due to this double scaling, the scaledUserDebt may be very large and if crvUSDBalance < scaledUserDebt, the function will revert with InsufficientBalance() error when in reality there may be enough tokens in the contract to cover the repay.

Tools Used

Manual Review

Recommendations

Remove the double scaling done in liquidateBorrower() and use the normalized value directly.

function liquidateBorrower(address userAddress) external onlyManagerOrOwner nonReentrant whenNotPaused {
---SNIP---
uint256 userDebt = lendingPool.getUserDebt(userAddress);
// @audit Remove this
- uint256 scaledUserDebt = WadRayMath.rayMul(userDebt, lendingPool.getNormalizedDebt());
​
if (userDebt == 0) revert InvalidAmount();
​
uint256 crvUSDBalance = crvUSDToken.balanceOf(address(this));
- if (crvUSDBalance < scaledUserDebt) revert InsufficientBalance();
+ if (crvUSDBalance < userDebt) revert InsufficientBalance();
​
// Approve the LendingPool to transfer the debt amount
- bool approveSuccess = crvUSDToken.approve(address(lendingPool), scaledUserDebt);
+ bool approveSuccess = crvUSDToken.approve(address(lendingPool), userDebt);
if (!approveSuccess) revert ApprovalFailed();
---SNIP---
}
Updates

Lead Judging Commences

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

StabilityPool::liquidateBorrower double-scales debt by multiplying already-scaled userDebt with usage index again, causing liquidations to fail

Support

FAQs

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

Give us feedback!