Summary
The initiateLiquidation function allows anyone to start the liquidation process for a user whose health factor falls below a certain threshold. However, the finalizeLiquidation function, which executes the actual liquidation, does not revalidate the user's health factor before proceeding. This creates a critical vulnerability: if a user deposits additional collateral (e.g., via depositNFT) after liquidation is initiated but before it is finalized, the user's health factor may improve above the liquidation threshold. Despite this, finalizeLiquidation would still proceed with the liquidation, causing significant and unjustified losses to the user.
Vulnerability Details
The initiateLiquidation function checks if the user is already under liquidation and initiates liquidation if the health factor is below the threshold.
function initiateLiquidation(address userAddress) external nonReentrant whenNotPaused {
if (isUnderLiquidation[userAddress]) revert UserAlreadyUnderLiquidation();
ReserveLibrary.updateReserveState(reserve, rateData);
UserData storage user = userData[userAddress];
uint256 healthFactor = calculateHealthFactor(userAddress);
if (healthFactor >= healthFactorLiquidationThreshold) revert HealthFactorTooLow();
isUnderLiquidation[userAddress] = true;
liquidationStartTime[userAddress] = block.timestamp;
emit LiquidationInitiated(msg.sender, userAddress);
}
However, the finalizeLiquidation function does not revalidate the user's health factor before proceeding with liquidation.
function finalizeLiquidation(address userAddress) external nonReentrant onlyStabilityPool {
if (!isUnderLiquidation[userAddress]) revert NotUnderLiquidation();
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;
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;
(uint256 amountScaled, uint256 newTotalSupply, uint256 amountBurned, uint256 balanceIncrease) = IDebtToken(reserve.reserveDebtTokenAddress).burn(userAddress, userDebt, reserve.usageIndex);
IERC20(reserve.reserveAssetAddress).safeTransferFrom(msg.sender, reserve.reserveRTokenAddress, amountScaled);
user.scaledDebtBalance -= amountBurned;
reserve.totalUsage = newTotalSupply;
ReserveLibrary.updateInterestRatesAndLiquidity(reserve, rateData, amountScaled, 0);
emit LiquidationFinalized(stabilityPool, userAddress, userDebt, getUserCollateralValue(userAddress));
}
If the user has deposited additional collateral after liquidation was initiated by calling depositNFT , their health factor may have improved above the threshold, making liquidation unnecessary and unfair.
POC
add following test case into LendingPool.test.js
it("liquidation should revert if the the total collateral value of a user increase", async function () {
const tokenId2 = 2;
await raacHousePrices.setHousePrice(2, ethers.parseEther("100"));
const amountToPay2 = ethers.parseEther("100");
await token.mint(user1.address, amountToPay2);
await token.connect(user1).approve(raacNFT.target, amountToPay2);
await raacNFT.connect(user1).mint(tokenId2, amountToPay2);
await raacHousePrices.setHousePrice(1, ethers.parseEther("90"));
await expect(lendingPool.connect(user2).initiateLiquidation(user1.address))
.to.emit(lendingPool, "LiquidationInitiated")
.withArgs(user2.address, user1.address);
expect(await lendingPool.isUnderLiquidation(user1.address)).to.be.true;
await raacNFT.connect(user1).approve(lendingPool.target, tokenId2);
await lendingPool.connect(user1).depositNFT(tokenId2);
const healthFactor = await lendingPool.calculateHealthFactor(user1.address);
const healthFactorLiquidationThreshold = await lendingPool.healthFactorLiquidationThreshold();
expect(healthFactor).to.be.gt(healthFactorLiquidationThreshold);
await ethers.provider.send("evm_increaseTime", [72 * 60 * 60 + 1]);
await ethers.provider.send("evm_mine");
await crvusd.connect(owner).mint(owner.address, ethers.parseEther("1000"));
await lendingPool.connect(owner).setStabilityPool(owner.address);
await lendingPool.connect(owner).finalizeLiquidation(user1.address);
});
run npx hardhat test --grep "liquidation should revert"
LendingPool
Liquidation
✔ liquidation should revert if the the total collateral value of a user increase (4406ms)
1 passing (25s)
We can see that the user is still liquidated even though the health factor is above the liquidation threshold.
Impact
-
Unjustified Liquidation:
-
Loss of Trust:
-
Financial Loss:
The impact is High, the likelihood is Low, so the severity is Medium.
Tools Used
Manual Review
Recommendations
To fix this issue, the finalizeLiquidation function should revalidate the user's health factor before proceeding with liquidation. Here is the corrected implementation:
function finalizeLiquidation(address userAddress) external nonReentrant onlyStabilityPool {
if (!isUnderLiquidation[userAddress]) revert NotUnderLiquidation();
ReserveLibrary.updateReserveState(reserve, rateData);
if (block.timestamp <= liquidationStartTime[userAddress] + liquidationGracePeriod) {
revert GracePeriodNotExpired();
}
uint256 healthFactor = calculateHealthFactor(userAddress);
if (healthFactor >= healthFactorLiquidationThreshold) {
revert HealthFactorAboveThreshold();
}
UserData storage user = userData[userAddress];
uint256 userDebt = user.scaledDebtBalance.rayMul(reserve.usageIndex);
isUnderLiquidation[userAddress] = false;
liquidationStartTime[userAddress] = 0;
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;
(uint256 amountScaled, uint256 newTotalSupply, uint256 amountBurned, uint256 balanceIncrease) =
IDebtToken(reserve.reserveDebtTokenAddress).burn(userAddress, userDebt, reserve.usageIndex);
IERC20(reserve.reserveAssetAddress).safeTransferFrom(msg.sender, reserve.reserveRTokenAddress, amountScaled);
user.scaledDebtBalance -= amountBurned;
reserve.totalUsage = newTotalSupply;
ReserveLibrary.updateInterestRatesAndLiquidity(reserve, rateData, amountScaled, 0);
emit LiquidationFinalized(stabilityPool, userAddress, userDebt, getUserCollateralValue(userAddress));
}
