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

Users can lose additional collateral by depositing NFTs after grace period expiration

patitonar

Summary

The LendingPool::depositNFT function does not check if a user is under liquidation and if their grace period has expired. This allows users to deposit additional NFTs in an attempt to increase their health factor, but if they fail to close the liquidation because of the grace period expiration, they will lose all collateral including the newly deposited NFTs.

Vulnerability Details

The LendingPool::depositNFT function lacks checks for:

  1. Whether the user is under liquidation (isUnderLiquidation[msg.sender])

  2. Whether their grace period has expired (block.timestamp > liquidationStartTime[msg.sender] + liquidationGracePeriod)

This allows users to deposit additional NFTs even when their position is already eligible for liquidation and after the grace period has expired, resulting in the loss of more collateral than necessary.

Impact

Users under liquidation can lose additional collateral by attempting to save their position after the grace period has expired. Since finalizeLiquidation() transfers all NFTs to the Stability Pool, any newly deposited NFTs will also be liquidated, causing users to lose more value than their original debt.

For example:

  1. User has 100k debt and 125k in NFT collateral

  2. Position becomes liquidatable and grace period expires

  3. User deposits additional 50k NFT trying to save position

  4. Liquidation executes, user loses 175k collateral to cover 100k debt

Tools Used

Manual review

Proof of Concept

Add the following test case to the test/unit/core/pools/LendingPool/LendingPool.test.js file in the Liquidation section:

it("should demonstrate loss of additional collateral after grace period", async function () {
// Set Stability Pool address (using owner for this test)
await lendingPool.connect(owner).setStabilityPool(owner.address);
await token.mint(owner.address, ethers.parseEther("100"));
​
// Decrease house price and initiate liquidation
await raacHousePrices.setHousePrice(1, ethers.parseEther("90"));
​
// Initiate liquidation
await lendingPool.connect(user2).initiateLiquidation(user1.address);
​
// Advance time beyond grace period (72 hours)
await ethers.provider.send("evm_increaseTime", [72 * 60 * 60 + 1]);
await ethers.provider.send("evm_mine");
​
// Verify the health factor is below the liquidation threshold
const healthFactor = await lendingPool.calculateHealthFactor(user1.address);
const healthFactorLiquidationThreshold = await lendingPool.healthFactorLiquidationThreshold();
expect(healthFactor).to.be.lt(healthFactorLiquidationThreshold);
​
// Mint new NFT to User1
const tokenId = 2;
await raacHousePrices.setHousePrice(tokenId, ethers.parseEther("100"));
const housePrice = await raacHousePrices.tokenToHousePrice(tokenId);
await token.mint(user1.address, housePrice);
await token.connect(user1).approve(raacNFT.target, housePrice);
await raacNFT.connect(user1).mint(tokenId, housePrice);
​
// Deposit new NFT to collateral
await raacNFT.connect(user1).approve(lendingPool.target, tokenId);
await lendingPool.connect(user1).depositNFT(tokenId);
​
// Verify the new health factor is below the liquidation threshold
const healthFactorAfterDeposit = await lendingPool.calculateHealthFactor(user1.address);
expect(healthFactorAfterDeposit).to.be.gt(healthFactorLiquidationThreshold);
​
// User1 is still under liquidation
expect(await lendingPool.isUnderLiquidation(user1.address)).to.be.true;
​
// User1 is not able to close liquidation
await expect(lendingPool.connect(user1).closeLiquidation())
.to.be.revertedWithCustomError(lendingPool, "GracePeriodExpired");
​
// Stability Pool closes liquidation
await expect(lendingPool.connect(owner).finalizeLiquidation(user1.address))
.to.emit(lendingPool, "LiquidationFinalized")
// Verify that the user is no longer under liquidation
expect(await lendingPool.isUnderLiquidation(user1.address)).to.be.false;
​
// Verify that the NFT has been transferred to the Stability Pool
expect(await raacNFT.ownerOf(1)).to.equal(owner.address);
expect(await raacNFT.ownerOf(tokenId)).to.equal(owner.address);
});

Recommendations

Add liquidation status checks to the depositNFT function:

function depositNFT(uint256 tokenId) external nonReentrant whenNotPaused {
+ if (isUnderLiquidation[msg.sender] &&
+ block.timestamp > liquidationStartTime[msg.sender] + liquidationGracePeriod) {
+ revert CannotDepositAfterGracePeriod();
+ }
// Rest of the function...
}
Updates

Lead Judging Commences

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

Users can deposit NFTs using LendingPool::depositNFT while under liquidation, leading to unfair liquidation of NFTs that weren't part of original position

Appeal created

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

Users can deposit NFTs using LendingPool::depositNFT while under liquidation, leading to unfair liquidation of NFTs that weren't part of original position

Support

FAQs

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