ParaSpace (Audit #3)

`validateEndAuction()` requires `isAuctioned(tokenId)`, however, doesn't require ```solidity require(collateralReserve.auctionStrategyAddress != address(0), Errors.AUCTION_NOT_ENABLED); ``` Theoretically, it can be turned off via a call to `PoolConfigurator.setReserveAuctionStrategyAddress()`.

The below `require` statement is invalid because the function caller can input the same address as the contract **POOL** to bypass the validation. Hence anyone can call the function `initialize()` to initialize the contract **NToken**. ```solidity=60 require(initializingPool == POOL, Errors.POOL_ADDRESSES_DO_NOT_MATCH); ```

In the function `getTokenPrice()`, the price from the ChainLink is used to calculate the amount0 and amount1 of Token0 and Token1. If the current price of ChainLink is out of the range of the position's lower and upper price, the amount0 or amount1 may be zero, which affects the price of the LP token finally. ```solidity=137 function getTokenPrice(uint256 tokenId) public view returns (uint256) { UinswapV3PositionData memory positionData = getOnchainPositionData( tokenId ); PairOracleData memory oracleData = _getOracleData(positionData);//ChainLink (uint256 liquidityAmount0, uint256 liquidityAmount1) = LiquidityAmounts .getAmountsForLiquidity( oracleData.sqrtPriceX96, TickMath.getSqrtRatioAtTick(positionData.tickLower), TickMath.getSqrtRatioAtTick(positionData.tickUpper), positionData.liquidity ); ( uint256 feeAmount0, uint256 feeAmount1 ) = getLpFeeAmountFromPositionData(positionData); return (((liquidityAmount0 + feeAmount0) * oracleData.token0Price) / 10**oracleData.token0Decimal) + (((liquidityAmount1 + feeAmount1) * oracleData.token1Price) / 10**oracleData.token1Decimal); } ``` **Proof of concept** - Case 1: Liquidate Asset 1. A liquidator can call the function `PoolCore.liquidateERC721()` to liquidate a borrower's non-healthy asset with Health Factor below 1. 2. Before the liquidation, the function `calculateUserAccountData()` calculates the total `xToken` balance of the user in the based currency used by the price oracle function `IPriceOracleGetter(oracle).getTokenPrice()`. 3. If the price obtained from ChainLink is at the left of `positionData.tickLower`, the number of token1 obtained will be 0. The price of the LP obtained according to the L158-L161 calculation logic may be lower than the price of the LP itself, which will eventually reduce the health factor of the asset. 4. In this case, the liquidator may successfully liquidate the asset that should be healthy. - Case 2: Borrow Asset 1. A user who provided collateral can call the function `PoolCore.borrow()` to borrow the ERC20 asset. 2. Before the borrowing, the function `calculateUserAccountData()` calculates the total `xToken` balance of the user in the based currency used by the price oracle function `IPriceOracleGetter(oracle).getTokenPrice()`. 3. If the price obtained from ChainLink is within the range of `positionData.tickLower` and `positionData.tickUpper`, but `currentTick` is exactly equal to `positionData.tickLower`(the number of token1 is 0), according to the calculation logic of L158-L161 the LP price may be higher than the actual price of LP, which will eventually increase the health factor of the asset. 4. In this case, the user may successfully borrow more ERC20 assets than the collateral.

The function `getReserveData()` is used to get the reserve data, however, there is no logical distinction between ERC20 and ERC721 assets. ```solidity=189 { DataTypes.ReserveData memory reserve = IPool( ADDRESSES_PROVIDER.getPool() ).getReserveData(asset); return ( reserve.accruedToTreasury, IERC20Detailed(reserve.xTokenAddress).totalSupply(), IERC20Detailed(reserve.variableDebtTokenAddress).totalSupply(), reserve.currentLiquidityRate, reserve.currentVariableBorrowRate, reserve.liquidityIndex, reserve.variableBorrowIndex, reserve.lastUpdateTimestamp ); } ```

Flash loans are a way to borrow large amounts of money for a certain fee. The requirement is that the loans need to be returned within the same transaction in a block. If not, the transaction will be reverted. An attacker can use the borrowed money as the initial funds for an exploit to enlarge the profit and/or manipulate the token price in the decentralized exchanges. We find that the function `getOnchainPositionData()` relies on price calculations based on the current sqrt price, meaning that they would be susceptible to flash-loan attacks by manipulating the price of given pairs to the attacker's benefit. ```solidity=71 (uint160 currentPrice, int24 currentTick, , , , , ) = pool.slot0(); ``` The result of this function could impact the results of the below functions: - getLiquidityAmount() - getLiquidityAmountFromPositionData() - getLpFeeAmount() - getLpFeeAmountFromPositionData() Two functions among them are used in the `UiPoolDataProvider.sol`. - getLiquidityAmountFromPositionData() - getLpFeeAmountFromPositionData() Although the current usage of the abovementioned functions is only on UI, we would like to remind the client to be cautious about the usage of these functions.

The linked codes do not affect the functionality of the codebase and appear to be either remnant of test code or older functionality.

The function `_safeTransfer()` does not invoke {IERC721Receiver-onERC721Received} on a target address, which is against the EIP-721 standard. ```solidity=320 function _safeTransfer( address from, address to, uint256 tokenId, bytes memory ) internal virtual { _transfer(from, to, tokenId); } ```

The contract is serving as the underlying entity to interact with third-party `UniswapV3`, `OpenSea`, `LooksRare`, `X2Y2`, `MoonBird`, `ChainLink`, `ApeCoinStaking`, `IEACAggregatorProxy`, `IAtomicPriceAggregator` and NFT Oracle protocols. The scope of the audit treats 3rd party entities as black boxes and assumes their functional correctness. However, in the real world, 3rd parties can be compromised and this may lead to lost or stolen assets. In addition, upgrades of 3rd parties can possibly create severe impacts, such as increasing fees of 3rd parties, migrating to new LP pools, etc. The `ParaSpace` protocol allows users to borrow assets using NFT as collateral. If NFT prices fluctuate significantly in the third-party markets, the Supplier's health factory may fluctuate as well. This is a potential risk to this protocol and to the Supplier.

The linked `transfer()`/`transferFrom()` invocations do not check the return value of the function call, which should yield `true` in the case of proper ERC-20 implementation.

The return value of an external call is not stored in a local or state variable.