Summary
The closeLiquidation() function reverts if called after the grace period, even if the user has repaid their debt or added sufficient collateral during the grace period. This allows the stability pool to liquidate healthy positions through finalizeLiquidation().
Vulnerability Details
In LendingPool::closeLiquidation() function checks if the grace period has expired before allowing users to close their liquidation:
function closeLiquidation() external nonReentrant whenNotPaused {
address userAddress = msg.sender;
if (!isUnderLiquidation[userAddress]) revert NotUnderLiquidation();
ReserveLibrary.updateReserveState(reserve, rateData);
@> if (block.timestamp > liquidationStartTime[userAddress] + liquidationGracePeriod) {
revert GracePeriodExpired();
}
UserData storage user = userData[userAddress];
uint256 userDebt = user.scaledDebtBalance.rayMul(reserve.usageIndex);
if (userDebt > DUST_THRESHOLD) revert DebtNotZero();
isUnderLiquidation[userAddress] = false;
liquidationStartTime[userAddress] = 0;
emit LiquidationClosed(userAddress);
}
However, this check is purely time-based and doesn't consider if the user has:
Repaid their debt through repay()
Added more collateral by depositing NFTs
Improved their health factor above the liquidation threshold
Even if a user takes any of these actions during the grace period to make their position healthy, they cannot close the liquidation if they call closeLiquidation() after the grace period expires. This allows the stability pool to call finalizeLiquidation() and seize the user's collateral despite the position being properly collateralized.
Impact
Users who successfully restore their position's health during the grace period but fail to call closeLiquidation() in time will:
Lose all their NFT collateral to the stability pool
Lose the repaid debt
Suffer significant financial losses even though their position was healthy
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("allows user to repay on time, but can't close liquidation after grace period, so he will be liquidated", async function () {
await lendingPool.connect(owner).setStabilityPool(owner.address);
await raacHousePrices.setHousePrice(1, ethers.parseEther("90"));
await lendingPool.connect(user2).initiateLiquidation(user1.address);
const userDebt = await lendingPool.getUserDebt(user1.address);
const debtTokenBalance = await debtToken.balanceOf(user1.address);
expect(debtTokenBalance).to.be.equal(userDebt);
const userBalanceBeforeRepay = await crvusd.balanceOf(user1.address);
const repayAmount = debtTokenBalance + ethers.parseEther("1");
await lendingPool.connect(user1).repay(repayAmount);
const userBalanceAfterRepay = await crvusd.balanceOf(user1.address);
const userDebtAfterRepay = await lendingPool.getUserDebt(user1.address);
const debtTokenBalanceAfterRepay = await debtToken.balanceOf(user1.address);
expect(userBalanceAfterRepay).to.be.lte(userBalanceBeforeRepay - debtTokenBalance);
expect(userBalanceAfterRepay).to.be.gte(userBalanceBeforeRepay - repayAmount);
expect(userDebtAfterRepay).to.be.equal(0);
expect(debtTokenBalanceAfterRepay).to.be.equal(0);
expect(await lendingPool.isUnderLiquidation(user1.address)).to.be.true;
await ethers.provider.send("evm_increaseTime", [72 * 60 * 60 + 1]);
await ethers.provider.send("evm_mine");
await expect(lendingPool.connect(user1).closeLiquidation())
.to.be.revertedWithCustomError(lendingPool, "GracePeriodExpired");
await expect(lendingPool.connect(owner).finalizeLiquidation(user1.address))
.to.emit(lendingPool, "LiquidationFinalized")
expect(await lendingPool.isUnderLiquidation(user1.address)).to.be.false;
expect(await raacNFT.ownerOf(1)).to.equal(owner.address);
});
Recommendations
The closeLiquidation() function should check the user's current health factor instead of only the grace period:
function closeLiquidation() external nonReentrant whenNotPaused {
address userAddress = msg.sender;
if (!isUnderLiquidation[userAddress]) revert NotUnderLiquidation();
// update state
ReserveLibrary.updateReserveState(reserve, rateData);
UserData storage user = userData[userAddress];
uint256 userDebt = user.scaledDebtBalance.rayMul(reserve.usageIndex);
+ // Allow closing if position is healthy, regardless of time
- if (block.timestamp > liquidationStartTime[userAddress] + liquidationGracePeriod) {
+ if (block.timestamp > liquidationStartTime[userAddress] + liquidationGracePeriod &&
+ calculateHealthFactor(userAddress) < healthFactorLiquidationThreshold) {
+ revert GracePeriodExpired();
+ }
if (userDebt > DUST_THRESHOLD) revert DebtNotZero();
isUnderLiquidation[userAddress] = false;
liquidationStartTime[userAddress] = 0;
emit LiquidationClosed(userAddress);
}
