Core Contracts

Summary

The withdrawNFT function in the LendingPool contract contains a critical flaw in its collateralization check. The current implementation uses percentMul instead of percentDiv when comparing the user's collateral value against their debt. This miscalculation can lead to undercollateralization, allowing users to withdraw NFTs even when their collateral value is insufficient to cover their debt, thereby risking the protocol's financial stability.

Vulnerability Details

The withdrawNFT function is designed to allow users to withdraw their deposited NFTs, but only if the withdrawal does not leave them undercollateralized. The function checks whether the remaining collateral value (after withdrawing the NFT) is sufficient to cover the user's debt. The check is performed using the following line:

Here, collateralValue is the total value of the user's deposited NFTs, nftValue is the value of the NFT being withdrawn, and userDebt is the user's total debt. The liquidationThreshold is a percentage (e.g., 80%) that represents the minimum collateral-to-debt ratio required to avoid liquidation.

The issue lies in the use of percentMul for the liquidationThreshold. The percentMul function multiplies the userDebt by the liquidationThreshold, which is less than 100% (e.g., 80%). This results in a value that is smaller than the actual debt. For example, if the user's debt is 100 units and the liquidationThreshold is 80%, userDebt.percentMul(liquidationThreshold) will return 80 units. This means the check is comparing the remaining collateral value against 80 units of debt, which is incorrect.

The current implementation can lead to undercollateralization because it allows users to withdraw NFTs even when their remaining collateral value is insufficient to cover their debt. This can result in the protocol being undercollateralized, increasing the risk of insolvency.

PoC

1. Alice deposits 2 NFTs into the LendingPool contract. The total collateral value of these NFTs is 200 units.

2. Alice borrows 100 units of the reserve asset, resulting in a debt of 100 units.

3. Alice attempts to withdraw one of her NFTs, which has a value of 100 units.

4. The withdrawNFT function checks if the remaining collateral value (200 - 100 = 100 units) is less than userDebt.percentMul(liquidationThreshold). Assuming the liquidationThreshold is 80%, this check compares 100 units against 80 units (100 * 80% = 80). Since $80 < 100$, the check passes, and Alice is allowed to withdraw the NFT.

5. After withdrawing the NFT, Alice's remaining collateral value is 100 units, but her debt is still 100 units. This leaves her with a collateral-to-debt ratio of 100%, which is below the required 125% (100 / 80%) or in other term's Alice's health factor is now below 1. The protocol is now undercollateralized.

Impact

The incorrect collateralization check in the withdrawNFT function can lead to undercollateralization, allowing users to withdraw NFTs even when their remaining collateral value is insufficient to cover their debt. This increases the risk of insolvency for the protocol, as it may not have enough collateral to cover outstanding debts in the event of a liquidation.

Tools Used

• Manual code review

Recommendations

Replace percentMul with percentDiv in the collateralization check: