Nemeos security record

The high dependency on the external oracle makes it the most critical module of the system. Any malfunction will affect the pool, putting the LP funds at risk. These are different examples of price manipulation: - An attacker can create their own NFT collection and do some wash trading on OpenSea to increase the floor price of the collection (note that such wash trading could be extended over a long period of time). Then they can act as a borrower in the Nemeos protocol to buy an NFT from this collection and get a loan to buy one or more NFTs from this collection they created, and then never pay back the loan(s). - An NFT community can organize to increase the price of their NFT listings (also removing the low-price listings that they want to sell but cannot find any buyers), increasing the floor price. Then they can buy multiple NFTs at the higher floor price, with funds from the Nemeos pool, and never pay it back. They would do this in order to increase the trading volume for that collection to get the attention of other buyers who might use their own funds to buy the lower-price listings from the attackers. As the collection is not in demand anymore, the NFT owners will get some profit, and no one will buy at liquidation.

The `Pool.totalAssets()` function employs a looping logic over the `_ongoingLiquidations` and `_ongoingLoans` arrays to calculate the total assets, posing a significant risk of high gas consumption. This design could lead to reaching the block gas limit, effectively causing a Denial of Service (DoS) condition for critical functions of the main pool. In smart contract development, especially on platforms like Ethereum where gas costs are a crucial consideration, it's essential to optimize for efficiency and minimize operations that require extensive computational resources. A similar issue applies to `Pool.onGoingLoans()`.

The `ModifiedERC4626Upgradeable.sol` file is identified as a direct copy of OpenZeppelin's original `ERC4626Upgradeable.sol`, with the sole differences being `_decimalsOffset()` and `_asset` from private to internal. This practice of cloning and slightly modifying foundational code from well-established libraries, such as OpenZeppelin, poses risks including maintenance overhead, potential for integration errors, and difficulties in tracking updates or security patches from the original source.

The `NFTFilter` contract currently implements a custom ownership model using a `protocolAdmin` variable to enforce owner-restricted operations. This custom approach to ownership and access control introduces extra complexity and potential security vulnerabilities, notably the absence of a mechanism to update the owner (admin) address. In scenarios where the admin address is compromised or needs to be securely changed for administrative reasons, the current implementation does not offer a direct or secure method to update the admin address.

In `for` loops iterating through an array, gas can be saved by caching the array length before starting the loop, instead of accessing it in every iteration.

In the `refundFromLiquidation` function, there is a code segment that attempts to transfer `refundSurplus` to `borrower_` without reverting if the transfer fails. While this approach avoids transaction reversion due to transfer failures, it is unclear how the funds should be handled in such a situation since the borrower funds are actually distributed between the shareholders.

The current system logic, which fixes interest rates at the time of NFT purchase from the collection, presents a significant risk to the sustainability of the lending protocol. This design does not incentivize borrowers to repay loans, especially in scenarios where liquidity becomes scarce. Unlike dynamic systems like Compound, where interest rates automatically adjust based on supply and demand—increasing significantly during liquidity shortages to incentivize loan repayments and attract more deposits—this protocol's fixed interest rate model fails to adapt to changing market conditions. Consequently, even if liquidity providers decide to increase interest rates, such adjustments will only affect future borrowers, leaving existing loans unaffected and potentially leading to a liquidity crisis without mechanisms to encourage loan repayment or attract new deposits.

In the current implementation of the `NFTWrapper` contract, a mechanism is in place to track pool addresses using an array. When determining if a pool exists within this array using `existsInPools()`, the contract iterates through all the pool addresses, performing an equality check on each address to identify a match. This approach raises potential concerns regarding gas efficiency and scalability. As the number of pools grows, the cost (in terms of gas) to iterate through this array and check each address increases linearly, which could lead to prohibitively high gas costs for users or the contract itself when interacting with a large set of pools.

`Pool` inherits from `ERC-4626` implementation from OpenZeppelin, but overrides some functions that make it non-ERC4626 compliant: - Preview functions can return more than the actual balance, and some withdrawals can fail (the received funds are less than expected). - The underlying token is not an ERC-20 token, but a native token. Consider using WETH (ERC-20 Wrapped ETH). This can cause issues if the `Pool` contract is intended to interact with other DeFi protocols.