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

A user can still be liquidated even after repaying the debt if they do not manually close the liquidation

avoloder

Summary

The liquidation process allows the Stability Pool to liquidate users after the grace period if the liquidation has not been closed, even if users have repaid their debt.

Vulnerability Details

After liquidation has been initiated, the user has a three-day grace period to repay the debt before the Stability Pool can finalize the liquidation. After repayment, the user must manually call closeLiquidation() to clear the liquidation status. However, in many lending protocols, it is common practice that users do not need to worry about the liquidation process once the debt is repaid.

In the current liquidation process, a problem arises if the user repays the debt but forgets to close the liquidation. Another scenario where the same issue occurs is if someone else repays the debt for the user, but the user was AFK or unaware of it. The person who repaid the debt cannot close the liquidation because the function uses msg.sender as the address. Technically, the debt is repaid, and the user should no longer be liquidatable. However, the flag isUnderLiquidation[userAddress] is only set to false after the liquidation is manually closed.

function closeLiquidation() external nonReentrant whenNotPaused {
address userAddress = msg.sender;
​
if (!isUnderLiquidation[userAddress]) revert NotUnderLiquidation();
​
-- SNIPET --
​
isUnderLiquidation[userAddress] = false;
liquidationStartTime[userAddress] = 0;
​
emit LiquidationClosed(userAddress);
}

After the three-day grace period expires, the Stability Pool calls finalizeLiquidation, and it succeeds because the user's liquidation status has not changed. Even though the user's debt is 0, the burn function of the DebtToken does not check the amount burned and "burns" 0 tokens. Before the token burning process, the user's entire collateral is transferred to the Stability Pool, leaving the user with no collateral and 0 debt.

function finalizeLiquidation(address userAddress) external nonReentrant onlyStabilityPool {
if (!isUnderLiquidation[userAddress]) revert NotUnderLiquidation();
​
-- SNIPET --
​
isUnderLiquidation[userAddress] = false;
liquidationStartTime[userAddress] = 0;
​
​
// Transfer NFTs to Stability Pool => takes all NFTs from the user
for (uint256 i = 0; i < user.nftTokenIds.length; i++) {
uint256 tokenId = user.nftTokenIds[i];
user.depositedNFTs[tokenId] = false;
raacNFT.transferFrom(address(this), stabilityPool, tokenId);
}
delete user.nftTokenIds;
​
// Burn DebtTokens from the user => will not revert, only 0 tokens will be burnt because userDebt = 0
(uint256 amountScaled, uint256 newTotalSupply, uint256 amountBurned, uint256 balanceIncrease) = IDebtToken(reserve.reserveDebtTokenAddress).burn(userAddress, userDebt, reserve.usageIndex);
​
-- SNIPET --
}

POC

Add this test to the LendingPool.test.js:

it("liquidates the user if user repays his debt without closing liquidation after grace period", async function () {
// Decrease house price to trigger liquidation
await raacHousePrices.setHousePrice(1, ethers.parseEther("90"));
​
// Initiate liquidation
await expect(lendingPool.connect(user2).initiateLiquidation(user1.address))
.to.emit(lendingPool, "LiquidationInitiated")
.withArgs(user2.address, user1.address);
// Verify that the user is under liquidation
expect(await lendingPool.isUnderLiquidation(user1.address)).to.be.true;
​
// User1 repays the debt right before grace period expiration but forgets to call `closeLiquidation()`
const userDebt = await lendingPool.getUserDebt(user1.address);
await crvusd.connect(user1).approve(lendingPool.target, userDebt + ethers.parseEther("1"));
await lendingPool.connect(user1).repay(userDebt + ethers.parseEther("1"));
​
// Verify that the user's debt is 0, and its collateral is equal to the initial amount
expect(await lendingPool.getUserDebt(user1.address)).to.be.eq(0);
expect(await lendingPool.getUserCollateralValue(user1.address)).to.be.eq(ethers.parseEther("90"));
​
// Advance time (beyond grace period)
await ethers.provider.send("evm_increaseTime", [72 * 60 * 60 + 1]);
await ethers.provider.send("evm_mine");
​
// Verify that the user is still under liquidation
expect(await lendingPool.isUnderLiquidation(user1.address)).to.be.true;
​
// Since the user is still under liquidation, StabilityPool tries to liquidate the user
await crvusd.connect(owner).mint(owner.address, ethers.parseEther("1000"));
await lendingPool.connect(owner).setStabilityPool(owner.address);
​
await expect(lendingPool.connect(owner).finalizeLiquidation(user1.address)).to.emit(lendingPool, "LiquidationFinalized");
​
// Verify that the user is not under liquidation anymore
expect(await lendingPool.isUnderLiquidation(user1.address)).to.be.false;
​
// Verify the loss of user's collateral
expect(await lendingPool.getUserCollateralValue(user1.address)).to.be.eq("0");
});

Impact

Likelihood: High; Impact: High
Any user can still be liquidated if the user repays the debt but does not call closeLiquidation()

Tools Used

Manual Review

Recommendations

There are two options that could be implemented to remove the responsibility of liquidation closure from the end users and prevent the situation described above from occurring:

Option 1:

Make the closeLiquidation() function internal, add the user address as a parameter, and then in the _repay() function, after debt repayment, check if the user is under liquidation and check the health factor to see if the user should still be liquidated. If not, call closeLiquidation() automatically.

function closeLiquidation(address user) internal whenNotPaused {
-- SNIPET --
};
​
function _repay(uint256 amount, address onBehalfOf) internal {
-- SNIPET --
// check if the user is under liquidation
if (isUnderLiquidation[onBehalfOf]) {
// check health factor again after debt repayment
uint256 healthFactor = calculateHealthFactor(onBehalfOf);
// if user not liquidatable anymore, close the liquidation
if (healthFactor >= healthFactorLiquidationThreshold) {
closeLiquidation(onBehalfOf);
}
}
-- SNIPET --
}

Rerun the test from the POC to confirm it fails now.

Option 2:

If the current implementation is your design choice, you should still implement a safety check in finalizeLiquidation() to see if the user's debt has already been fully repaid and gracefully return

Updates

Lead Judging Commences

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

LendingPool::finalizeLiquidation() never checks if debt is still unhealthy

A borrower can LendingPool::repay to avoid liquidation but might not be able to call LendingPool::closeLiquidation successfully due to grace period check, loses both funds and collateral

Support

FAQs

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