Core Contracts

Summary

Context: LendingPool.sol#L307

The withdrawNFT function in the LendingPool contract is vulnerable to a potential Denial of Service (DoS) attack due to the unbounded loop in the process of finding and removing a specified tokenId from the user's deposited NFTs array. If the user.nftTokenIds array is large, the loop can exceed the gas limit, preventing users from withdrawing their NFTs, potentially causing a permanent blockage of their funds.

Vulnerability Details

In the depositNFT function, the contract allows users to add an NFT's tokenId to their user.nftTokenIds array. However, as the number of NFTs deposited by a user increases, the array becomes larger, and this may lead to an issue in the withdrawNFT function. Specifically, the unbounded loop in withdrawNFT that searches for a tokenId in the user.nftTokenIds array could exceed the gas limit if the array is too long.

Impact

Users who deposit too many NFTs into the LendingPool may experience issues withdrawing them later. If the user.nftTokenIds array grows too large, the withdrawNFT function may fail due to gas limitations, preventing users from withdrawing their NFTs. This could cause a denial of service for the affected user.

Tools Used

Manual review

Recommendations

To prevent the issue of exceeding gas limits due to large dynamic arrays, it is recommended to replace the use of the dynamic array user.nftTokenIds with a mapping variable to track which token IDs have been deposited. This way, the contract can efficiently check and manage deposited NFTs without the overhead of looping through an array.

For example, use a mapping like:

When an NFT is deposited, set the corresponding token ID as true in the mapping:

This eliminates the need for the for-loop that searches through the user.nftTokenIds array, improving gas efficiency and preventing denial of service (DOS) attacks due to exceeding the gas limit.