Core Contracts

Summary

LendingPool::withdrawNFT function is susceptible to revert due to OOG in the case that the user has a lot of NFTs deposited, making it then impossible for the user to redeem their own NFTs, making them stuck forever in the protocol, thus breaking protocol solvency.

Details

LendingPool::withdrawNFT contains a loop where it iterates through the user NFTs and implements switch-and-pop logic to remove withdrawn NFT from the protocol state:

However, this approach is not sound. Consider the following scenario:

Since NFTs are house NFTs, imagine a multisig collective or a business buying lots of houses (NFTs), I’m underlining this because in the logic of the protocol, a single person owning a lot of NFTs (houses) might not be that feasible, but in the case of a collective or a business that rents houses, it is quite feasible to imagine that to happen. So, the collective buys a lot of NFTs and interacts with the LendingPool contract for this or that reason. Then, they want to withdraw one of their NFTs, due to OOG, they won’t be able to, never ever.

Never and ever. Their NFTs, all of them, will be forever stuck in the pool.

Impact

In the described case, user NFTs will be forever stuck in the pool, impossible to redeem. That is unacceptable in a protocol like this one.

Recommendation

Instead of looping, you can change the way how you store nft ids to a mapping, not an array, so that you can directly get the relevant nft.