Core Contracts

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

Double Multiplication of Normalized Debt Results in Overestimation of User Debt During Liquidation

Summary

StabilityPool::liquidateBorrower incorrectly calculates a user's total debt by multiplying the normalized debt by the usage index twice. This results in a significant overestimation of the amount required for liquidation, leading to unnecessary revert in StabilityPool::liquidateBorrower

Vulnerability Details

In the liquidateBorrower function, the contract retrieves the user’s debt using:

'''solidity
uint256 userDebt = lendingPool.getUserDebt(userAddress);

This function already applies the usage index to the users scaled debt:
```solidity
function getUserDebt(address userAddress) public view returns (uint256) {
UserData storage user = userData[userAddress];
return user.scaledDebtBalance.rayMul(reserve.usageIndex);
}

However, in StabilityPool::liquidateBorrower, the contract incorrectly multiplies userDebt by the normalized debt (usage index) again:

uint256 scaledUserDebt = WadRayMath.rayMul(
userDebt,
lendingPool.getNormalizedDebt()
);

Proof Of Code (POC)

This test was run in protocols-test.js in the "StabilityPool" describe block

it("userdebtisdoublecounted", async function () {
//c for testing purposes
// Setup stability pool deposit
// Create position to be liquidated
const newTokenId = HOUSE_TOKEN_ID + 2;
await contracts.housePrices.setHousePrice(newTokenId, HOUSE_PRICE);
await contracts.crvUSD
.connect(user2)
.approve(contracts.nft.target, HOUSE_PRICE);
await contracts.nft.connect(user2).mint(newTokenId, HOUSE_PRICE);
await contracts.nft
.connect(user2)
.approve(contracts.lendingPool.target, newTokenId);
await contracts.lendingPool.connect(user2).depositNFT(newTokenId);
await contracts.lendingPool.connect(user2).borrow(BORROW_AMOUNT);
// Trigger and complete liquidation
await contracts.housePrices.setHousePrice(
newTokenId,
(HOUSE_PRICE * 10n) / 100n
);
//c need to increase time to allow usage index to update
await time.increase(73 * 60 * 60);
await contracts.lendingPool
.connect(user3)
.initiateLiquidation(user2.address);
await time.increase(73 * 60 * 60);
// Will call the lendingPool.finalizeLiquidation(user2.address)
//c get the user debt before liquidation. need to update state to update the usage index
contracts.lendingPool.updateState();
const userdebt = await contracts.lendingPool.getUserDebt(user2.address);
console.log(`userdebt: ${userdebt}`);
const reservedata1 = await contracts.lendingPool.getAllUserData(
user2.address
);
console.log(
`reservedata1.scaledDebtBalance: ${reservedata1.scaledDebtBalance}`
);
//c IMPORTANT: to get this to display the correct userscaleddebtbalance, you need to go into lendingpool::getalluserdata and change the scaledDebtBalance return value to user.scaledDebtBalance instead of getUserDebt(userAddress)
console.log(`usageindex: ${reservedata1.usageIndex}`);
const expecteddebt = await contracts.reserveLibrary.raymul(
reservedata1.scaledDebtBalance,
reservedata1.usageIndex
);
console.log(`expecteddebt: ${expecteddebt}`);
//c send sufficient amount of crvUSD to the stability pool to cover the expected debt
await contracts.crvUSD
.connect(user3)
.approve(contracts.stabilityPool.target, STABILITY_DEPOSIT);
await contracts.crvUSD
.connect(user3)
.transfer(contracts.stabilityPool.target, expecteddebt);
/*c IMPORTANT: for this test to work, first go to reservelibrarymock.sol and include the following function:
function raymul(
uint256 val1,
uint256 val2
) external pure returns (uint256) {
return val1.rayMul(val2);
}
go into deploycontracts.js and add the following line:
//c deploy reservelibrarymock
const reserveLibrary = await deployContract("ReserveLibraryMock", []);
and add reserveLibrary to the return statement of deployContracts.js
*/
//c proof that the userdebt is greater than their borrow amount which proves that the usage index has already been applied to it
assert(userdebt > BORROW_AMOUNT);
//c get normalized debt
const normalizeddebt = await contracts.lendingPool.getNormalizedDebt();
console.log(`normalizeddebt: ${normalizeddebt}`);
await contracts.lendingPool.connect(owner).updateState();
await expect(
contracts.stabilityPool
.connect(owner)
.liquidateBorrower(user2.address)
).to.be.revertedWithCustomError(
contracts.stabilityPool,
"InsufficientBalance"
);
});

To see the inflated debt, go into StabilityPool.sol and add the scaledDebt variable to the InsufficientBalance custom error and when you run this test, you will see that the debt returned is more than the expected debt.

Incorrect debt calculation – The debt amount is multiplied by the usage index twice, leading to a higher-than-actual debt value.
Excessive repayment during liquidation – The contract will require more crvUSD than necessary to settle the user's debt.

Impact

Stability pool will pay more than required to settle a borrower's debt. The lending protocol’s fund allocations will become inaccurate, leading to mismanagement of reserves as it will always have to repay more than the user's actual debt. In extreme cases, systemic risks could arise due to miscalculated liquidations affecting protocol stability.

Tools Used

Manual Review, Hardhat

Recommendations

Remove the extra multiplication by the usage index in liquidateBorrower. Instead of:

uint256 scaledUserDebt = WadRayMath.rayMul(
userDebt,
lendingPool.getNormalizedDebt()
);

Use:

uint256 scaledUserDebt = userDebt;

Since LendingPool::getUserDebt() already applies the usage index, this change ensures the debt amount is accurately calculated.

Updates

Lead Judging Commences

inallhonesty Lead Judge 3 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.