Core Contracts

Regnum Aurum Acquisition Corp
HardhatReal World AssetsNFT
77,280 USDC
View results
Submission Details
Severity: high
Valid

Incorrect collateral safety check in LendingPool::withdrawNFT allows undercollateralized positions

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:

// LendingPool.sol
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 () {
// Mint second NFT
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"));
// Deposit 2 NFTs
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);
// Borrow 100 crvUSD (safe with 200 crvUSD collateral)
await lendingPool.connect(user1).borrow(ethers.parseEther("100"));
// Withdraw 1 NFT (remaining collateral 100 crvUSD) - should revert due to undercollaterization but did not
// 200 - 100 < 100 * 80% -> results in false
await expect(lendingPool.connect(user1).withdrawNFT(1)).to.not.be.reverted;
// Verify undercollateralized position
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();
}
Updates

Lead Judging Commences

inallhonesty Lead Judge 3 months ago
Submission Judgement Published
Validated
Assigned finding tags:

LendingPool::borrow as well as withdrawNFT() reverses collateralization check, comparing collateral < debt*0.8 instead of collateral*0.8 > debt, allowing 125% borrowing vs intended 80%

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.