Summary
Users are liquidated even if their health factor is greater than healthFactorLiquidationThreshold due to missing user health factor check in finalizeLiquidation and closeLiquidation.
Vulnerability Details
If user healthFactor < healthFactorLiquidationThreshold, the liquidation is initiated.
/contracts/core/pools/LendingPool/LendingPool.sol:466
466: uint256 healthFactor = calculateHealthFactor(userAddress);
467: if (healthFactor >= healthFactorLiquidationThreshold) revert HealthFactorTooLow();
Now the grace period starts. if user payback loan or deposit an other nft (collatoral), they are suppose to call closeLiquidation to stop Liquidation.
/contracts/core/pools/LendingPool/LendingPool.sol:509
509:
510: if (block.timestamp > liquidationStartTime[userAddress] + liquidationGracePeriod) {
511: revert GracePeriodExpired();
512: }
User health factor is calculated as follows:
/contracts/core/pools/LendingPool/LendingPool.sol:554
554: function calculateHealthFactor(address userAddress) public view returns (uint256) {
555: uint256 collateralValue = getUserCollateralValue(userAddress);
556: uint256 userDebt = getUserDebt(userAddress);
557: if (userDebt < 1) return type(uint256).max;
558: uint256 collateralThreshold = collateralValue.percentMul(liquidationThreshold);
559: return (collateralThreshold * 1e18) / userDebt;
560: }
However, when finalizing liquidation, there is no health factor check, meaning the user is still liquidated even if they regain a healthy position.
Additionally, closeLiquidation and finalizeLiquidation only checks userDebt, not the healthFactor, preventing users from stopping liquidation even if their health factor improves.
/contracts/core/pools/LendingPool/LendingPool.sol:490
490: uint256 userDebt = user.scaledDebtBalance.rayMul(reserve.usageIndex);
491: if (userDebt > DUST_THRESHOLD) revert DebtNotZero();
POC
Add this POC to LendingPool.test.js in describe("Liquidation", section:
/test/unit/core/pools/LendingPool/LendingPool.test.js:847
847:
848: it.only("should allow the user to close liquidation within grace period", async function () {
849: await raacHousePrices.setHousePrice(1, ethers.parseEther("90"));
850: await lendingPool.connect(user2).initiateLiquidation(user1.address);
851: const healthFactorLiquidationThreshold = await lendingPool.healthFactorLiquidationThreshold();
852: console.log("healthFactorLiquidationThreshold",healthFactorLiquidationThreshold)
853: const userDebt = await lendingPool.getUserDebt(user1.address);
854: console.log("userDebt",userDebt)
855: const healthFactorBefore = await lendingPool.calculateHealthFactor(user1.address)
856: console.log("healthFactorBefore",healthFactorBefore)
857: expect(healthFactorLiquidationThreshold).to.gt(healthFactorBefore);
858: const tokenId = 2;
859: const amountToPay = ethers.parseEther("100");
860:
861: await token.mint(user1.address, amountToPay);
862:
863: await token.connect(user1).approve(raacNFT.target, amountToPay);
864: await raacHousePrices.setHousePrice(2, ethers.parseEther("100"));
865:
866: await raacNFT.connect(user1).mint(tokenId, amountToPay);
867:
868:
869: await raacNFT.connect(user1).approve(lendingPool.target, tokenId);
870: await lendingPool.connect(user1).depositNFT(tokenId);
871:
872: await lendingPool.connect(owner).setStabilityPool(owner.address);
873:
874:
875: await expect(lendingPool.connect(user1).closeLiquidation())
876: .to.be.reverted
877:
878: const healthFactorAfter = await lendingPool.calculateHealthFactor(user1.address)
879:
880: console.log("healthFactorAfter",healthFactorAfter)
881: const healthFactorLiquidationThresholdAfter = await lendingPool.healthFactorLiquidationThreshold();
882: console.log("healthFactorLiquidationThresholdAfter",healthFactorLiquidationThresholdAfter)
883:
884: expect(healthFactorLiquidationThresholdAfter).to.lt(healthFactorAfter);
885:
886:
887: await ethers.provider.send("evm_increaseTime", [72 * 60 * 60 + 1]);
888: await ethers.provider.send("evm_mine");
889:
890: await crvusd.connect(owner).mint(owner.address, ethers.parseEther("1000"));
891:
892:
893:
894: await lendingPool.connect(owner).finalizeLiquidation(user1.address)
895:
896: });
and run this npx hardhat test
Impact
Users cannot stop liquidation even if they restore their health factor and liquidation proceeds regardless of improved collateral position.
Tools Used
Manual Review, Unit Testing
Recommendations
Add healthFactor checks in closeLiquidation and finalizeLiquidation.
/contracts/core/pools/LendingPool/LendingPool::closeLiquidation
- UserData storage user = userData[userAddress];
- uint256 userDebt = user.scaledDebtBalance.rayMul(reserve.usageIndex);
- if (userDebt > DUST_THRESHOLD) revert DebtNotZero();
+ uint256 healthFactor = calculateHealthFactor(userAddress);
+ if (healthFactor < healthFactorLiquidationThreshold) revert HealthFactorTooLow();
/contracts/core/pools/LendingPool/LendingPool::finalizeLiquidation
+ uint256 healthFactor = calculateHealthFactor(userAddress);
+ if (healthFactor >= healthFactorLiquidationThreshold) revert HealthFactorTooLow();
