Summary
Users may be unable to close liquidation after repaying their loan due to grace period timing.
Vulnerability Details
When a user's loan becomes unhealthy, anyone can initiate the liquidation using initiateLiquidation. After the liquidation is initiated, there's a grace period that allows the user to repay their loan before the liquidation is finalized. For this to happen, the user must first call repay and then call closeLiquidation.
There is a check inside closeLiquidation that prevents closing the liquidation if the grace period has expired:
if (block.timestamp > liquidationStartTime[userAddress] + liquidationGracePeriod) {
revert GracePeriodExpired();
}
This could be problematic if the user repays their loan close to the end of the grace period. If the repayment happens near the expiration of the grace period and the network is busy, it might take a while for the transaction to be mined. If the block.timestamp surpasses the grace period, the user's call to closeLiquidation will revert, even though the loan has been repaid.
Additionally, many other actions, like withdrawNFT, check the liquidation status of the user's loan:
function withdrawNFT(uint256 tokenId) external nonReentrant whenNotPaused {
if (isUnderLiquidation[msg.sender]) revert CannotWithdrawUnderLiquidation();
As a result, the user won't be able to withdraw their NFT even after repaying their loan, leaving the NFT stuck in the lendingPool.
Proof of Concept
To test the scenario, please add this test case to LendingPool.test.js under Liquidation suite:
it("User can't close the liquidation even after repayment", async function () {
const newValue = 2 * 24 * 60 * 60;
await lendingPool.connect(owner).setParameter(2, newValue);
expect(await lendingPool.liquidationGracePeriod()).to.equal(newValue);
await raacHousePrices.setHousePrice(1, ethers.parseEther("90"));
await lendingPool.connect(user2).initiateLiquidation(user1.address);
await ethers.provider.send("evm_increaseTime", [2 * 24 * 60 * 60]);
await ethers.provider.send("evm_mine");
const userDebt = await lendingPool.getUserDebt(user1.address);
await crvusd.connect(user1).approve(lendingPool.target, userDebt + ethers.parseEther("1"));
await lendingPool.connect(user1).repay(userDebt + ethers.parseEther("1"));
await ethers.provider.send("evm_increaseTime", [1]);
await ethers.provider.send("evm_mine");
await expect(lendingPool.connect(user1).closeLiquidation())
.to.be.revertedWithCustomError(lendingPool, "GracePeriodExpired");
expect(await lendingPool.isUnderLiquidation(user1.address)).to.be.true;
await expect(lendingPool.connect(user1).withdrawNFT(1))
.to.be.revertedWithCustomError(lendingPool, "CannotWithdrawUnderLiquidation");
expect(await raacNFT.ownerOf(1)).to.equal(lendingPool.target);
const userData = await lendingPool.userData(user1.address);
expect(userData.scaledDebtBalance).to.equal(0);
expect(userData.nftTokenIds).to.be.equal(undefined);
const userClosedLiquidationDebt = await lendingPool.getUserDebt(user1.address);
expect(userClosedLiquidationDebt).to.equal(0);
const healthFactor = await lendingPool.calculateHealthFactor(user1.address);
expect(healthFactor).to.equal(ethers.MaxUint256);
});
Run the test:
npm run test:unit:pools:lendingpool -- --grep "User can't close the liquidation even after repayment"
Impact
The user's NFTs may become stuck in the lendingPool, potentially resulting in the loss of funds.
Tools Used
Recommendations
Instead of having a separate function to close the liquidation, I recommend closing the liquidation automatically when the user successfully repays their loan:
function _repay(uint256 amount, address onBehalfOf) internal {
if (amount == 0) revert InvalidAmount();
if (onBehalfOf == address(0)) revert AddressCannotBeZero();
UserData storage user = userData[onBehalfOf];
// Update reserve state before repayment
ReserveLibrary.updateReserveState(reserve, rateData);
// Calculate the user's debt (for the onBehalfOf address)
uint256 userDebt = IDebtToken(reserve.reserveDebtTokenAddress).balanceOf(onBehalfOf);
uint256 userScaledDebt = userDebt.rayDiv(reserve.usageIndex);
// If amount is greater than userDebt, cap it at userDebt
uint256 actualRepayAmount = amount > userScaledDebt ? userScaledDebt : amount;
uint256 scaledAmount = actualRepayAmount.rayDiv(reserve.usageIndex);
// Burn DebtTokens from the user whose debt is being repaid (onBehalfOf)
// is not actualRepayAmount because we want to allow paying extra dust and we will then cap there
(uint256 amountScaled, uint256 newTotalSupply, uint256 amountBurned, uint256 balanceIncrease) =
IDebtToken(reserve.reserveDebtTokenAddress).burn(onBehalfOf, amount, reserve.usageIndex);
// Transfer reserve assets from the caller (msg.sender) to the reserve
IERC20(reserve.reserveAssetAddress).safeTransferFrom(msg.sender, reserve.reserveRTokenAddress, amountScaled);
reserve.totalUsage = newTotalSupply;
user.scaledDebtBalance -= amountBurned;
// Update liquidity and interest rates
ReserveLibrary.updateInterestRatesAndLiquidity(reserve, rateData, amountScaled, 0);
emit Repay(msg.sender, onBehalfOf, actualRepayAmount);
+ userDebt = user.scaledDebtBalance.rayMul(reserve.usageIndex);
+
+ if (userDebt > DUST_THRESHOLD) revert DebtNotZero();
+
+ isUnderLiquidation[onBehalfOf] = false;
+ liquidationStartTime[onBehalfOf] = 0;
+
+ emit LiquidationClosed(onBehalfOf);
}
