Core Contracts

Regnum Aurum Acquisition Corp

HardhatReal World AssetsNFT

77,280 USDC

View results

Previous Next

Submission Details

Severity: high

Valid

`LendingPool::finalizeLiquidation` - Excess Collateral Not Returned to User in Case of NFT Value Increase or New Deposits

federodes

Summary

In the finalizeLiquidation function of the LendingPool contract, if the value of the liquidated NFTs exceeds the debt owed by the user, the excess value is not returned to the user. Instead, any surplus collateral is transferred to the stabilityPool, leading to potential financial loss for over-collateralized users.

Vulnerability Details

The current implementation of finalizeLiquidation does not check if the total value of the liquidated NFTs exceeds the user's debt. As a result, any excess collateral is not refunded to the user but is instead transferred to the stabilityPool. While the liquidation process ensures the user’s debt is cleared, it lacks a mechanism to return surplus collateral.

Excess collateral can arise in two ways between the initiation and finalization of liquidation. If the value of an NFT increases after liquidation has started but before it is finalized, the position could become over-collateralized again. However, the contract does not account for this increase and still liquidates the NFTs. Additionally, there is no restriction preventing a user from depositing new NFTs after liquidation has been initiated. The depositNFT function lacks a check to prevent deposits when a user is flagged for liquidation. As a result, newly deposited NFTs can be lost, as they are transferred to the stabilityPool upon liquidation, even if they were sufficient to restore a healthy collateral position.

Affected Code

`depositNFT` Function:

function depositNFT(uint256 tokenId) external nonReentrant whenNotPaused {

// update state

ReserveLibrary.updateReserveState(reserve, rateData);

if (raacNFT.ownerOf(tokenId) != msg.sender) revert NotOwnerOfNFT();

// @audit-issue Missing check for isUnderLiquidation

UserData storage user = userData[msg.sender];

if (user.depositedNFTs[tokenId]) revert NFTAlreadyDeposited();

user.nftTokenIds.push(tokenId);

user.depositedNFTs[tokenId] = true;

raacNFT.safeTransferFrom(msg.sender, address(this), tokenId);

emit NFTDeposited(msg.sender, tokenId);

}

Since the function does not verify whether the user is in isUnderLiquidation mode, users can deposit new NFTs, which will be lost upon liquidation.

`finalizeLiquidation` Function:

function finalizeLiquidation(address userAddress) external nonReentrant onlyStabilityPool {

if (!isUnderLiquidation[userAddress]) revert NotUnderLiquidation();

// 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);

isUnderLiquidation[userAddress] = false;

liquidationStartTime[userAddress] = 0;

// Transfer NFTs to Stability Pool

for (uint256 i = 0; i < user.nftTokenIds.length; i++) {

uint256 tokenId = user.nftTokenIds[i];

user.depositedNFTs[tokenId] = false;

raacNFT.transferFrom(address(this), stabilityPool, tokenId);

}

// @audit-issue Liquidates all NFTs regardless of increased value or new deposits

delete user.nftTokenIds;

// Burn DebtTokens from the user

(uint256 amountScaled, uint256 newTotalSupply, uint256 amountBurned, uint256 balanceIncrease) =

IDebtToken(reserve.reserveDebtTokenAddress).burn(userAddress, userDebt, reserve.usageIndex);

// Transfer reserve assets from Stability Pool to cover the debt

IERC20(reserve.reserveAssetAddress).safeTransferFrom(msg.sender, reserve.reserveRTokenAddress, amountScaled);

// Update user's scaled debt balance

user.scaledDebtBalance -= amountBurned;

reserve.totalUsage = newTotalSupply;

// Update liquidity and interest rates

ReserveLibrary.updateInterestRatesAndLiquidity(reserve, rateData, amountScaled, 0);

// @audit-issue If token price increases after liquidation starts, excess value is never returned to the user

emit LiquidationFinalized(stabilityPool, userAddress, userDebt, getUserCollateralValue(userAddress));

}

PoC

Add the following test to test/unit/core/pools/LendingPool/LendingPool.test.js.

A test demonstrating that an increase in NFT price after liquidation starts does not prevent liquidation
and does not refund excess value.

it.only("should returns excess crvUSD after closed liquidation", async function () {

const crvUSDBalanceBefore = await crvusd.balanceOf(user1.address);

const rTokenBalanceBefore = await rToken.balanceOf(user1.address);

console.log({crvUSDBalanceBefore, rTokenBalanceBefore})

// Decrease house price and initiate liquidation

// FIXME: we are using price oracle and therefore the price should be changed from the oracle.

await raacHousePrices.setHousePrice(1, ethers.parseEther("90"));

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");

// Fund the stability pool with crvUSD

await crvusd.connect(owner).mint(owner.address, ethers.parseEther("1000"));

// Set Stability Pool address (using owner for this test)

await lendingPool.connect(owner).setStabilityPool(owner.address);

// Simulate a sudden increase in price for the house

await raacHousePrices.setHousePrice(1, ethers.parseEther("100"));

await expect(lendingPool.connect(owner).finalizeLiquidation(user1.address))

.to.emit(lendingPool, "LiquidationFinalized")

const crvUSDBalanceAfter = await crvusd.balanceOf(user1.address);

const rTokenBalanceAfter = await rToken.balanceOf(user1.address);

console.log({crvUSDBalanceBefore, rTokenBalanceBefore})

console.log({crvUSDBalanceAfter, rTokenBalanceAfter})

// Verify that the user haven't received any compensation

expect(crvUSDBalanceBefore).to.eq(crvUSDBalanceAfter)

expect(rTokenBalanceBefore).to.eq(rTokenBalanceAfter)

});

Impact

Over-collateralized users risk losing excess value since the liquidation process does not refund surplus collateral. The stability pool benefits at the expense of liquidated users, reducing fairness in the system. NFTs deposited after liquidation begins are lost without benefiting the user.

Recommendations

To address these issues, calculate and refund any excess value before liquidating the user's NFTs. This can be achieved by summing the value of all NFTs and returning any surplus beyond the debt owed. Additionally, prevent deposits during liquidation by adding a check in depositNFT to revert if isUnderLiquidation[userAddress] is true. Lastly, improve liquidation logic by re-evaluating collateral before liquidation is executed. By implementing these changes, the liquidation process will be more fair and transparent, ensuring that users do not lose excess collateral unnecessarily.

Updates

Lead Judging Commences

inallhonesty Lead Judge 7 months ago

Submission Judgement Published

Validated

Assigned finding tags:

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

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

Prize pool breakdown

Total prize

77,280 USDC

nSLOC

3,864

20 USDC / LOC

High Medium

74,000

Low

3,280

Live

The contest is live. Earn rewards by submitting a finding.

Judging

Submissions are being carefully reviewed by our judges.

View all submissions

Appeals

This is your time to appeal against judgements on your submissions.

Appeals Review

Appeals are being carefully reviewed by our judges.

Rewards Distribution

The contest is complete and the rewards are being distributed.