ParaSpace - NFT Money Market

```solidity=286 require(owner == ecrecover(digest, v, r, s), Errors.INVALID_SIGNATURE); _nonces[owner] = currentValidNonce + 1; _approve(spender, value); ``` `NToken.permit()` allows the `spender` to transfer in the future the token with id `value` if correctly signed by `owner` message provided. However, it is not checked that `_isApprovedOrOwner(owner, value)`. As a result, anyone can get approval for any token. The `value` argument name is misleading.

The following check is performed when borrowing if the interest rate mode is stable: ```solidity require( !params.userConfig.isUsingAsCollateral( reservesData[params.asset].id ) || params.reserveCache.reserveConfiguration.getLtv() == 0 || params.amount > IToken(params.reserveCache.xTokenAddress).balanceOf( params.userAddress ), Errors.COLLATERAL_SAME_AS_BORROWING_CURRENCY ); ``` So the current attack only applies if the interest rate mode is variable. The following slightly tweaked attack works in either interest rate mode. Attack Flow: 1. An exploiter creates two contracts A and B. 2. Contract A flash loans 100 ETH from other protocols, it then supplies 100 ETH and is minted 100 ETH of PToken (assuming the asset is ETH), they then borrow 80% of the locked ETH in some other Token. (assuming up to 80% can be borrowed). 3. Contract A transfers the borrowed Token's to Contract B, which then swaps them for around 80 ETH. 4. Contract B then supplies 80 ETH and is minted 80 ETH worth of PToken. Subsequently, they borrow 64 ETH worth of some other Token, which they then swap for around 64 ETH. 5. Contract B uses its own 80 ETH worth of PToken to pay off Contract A's debt. 6. Contract A withdraws all 100 ETH supplied by burning its 100 ETH of PToken, as its debt is now cleared. 7. Contract A repays the 100 ETH flash loan. 9. The exploiter gains around 64 ETH in profit as they still have the 64 ETH in contract B. (Not accounting for the swap and flash loan fees.)

A reentrancy attack can occur when the contract creates a function that makes an external call to another untrusted contract before resolving any effects. If the attacker can control the untrusted contract, they can make a recursive call back to the original function, repeating interactions that would have otherwise not run after the external call resolved the effects. For example, the hacker can call the `withdrawERC721()` method and use the hook `_checkOnERC721Received` method of the ERC721 receiver to reenter the method `liquidationERC721()`.

```solidity=1 function batchBuyWithCredit( bytes32[] calldata marketplaceIds, bytes[] calldata payloads, DataTypes.Credit[] calldata credits, address onBehalfOf, uint16 referralCode ) external payable virtual override nonReentrant { ``` ```solidity function buyWithCredit( bytes32 marketplaceId, bytes calldata payload, DataTypes.Credit calldata credit, address onBehalfOf, uint16 referralCode ) external payable virtual override nonReentrant { ``` Both of the `buyWithCredit()` and `batchBuyWithCredit()` are external functions without any validation. Any user can call them with any parameter. At the same time, these functions do not validate the caller's allowance from the `onBehalfOf` account. Not verifying that the caller has allowance from the `onBehalfOf` account could be a vulnerability.

When transferring deflationary ERC20 tokens, the input amount may not be equal to the received amount due to the charged transaction fee. For example, if a user sends 100 deflationary tokens (with a 10% transaction fee), only 90 tokens actually arrived at the contract. However, a failure to discount such fees may allow the same user to withdraw 100 tokens from the contract, which causes the contract to lose 10 tokens in such a transaction. Reference: https://thoreum-finance.medium.com/what-exploit-happened-today-for-gocerberus-and-garuda-also-for-lokum-ybear-piggy-caramelswap-3943ee23a39f ```solidity=200 IERC20(token0).safeTransferFrom(sender, address(this), amountAdd0); ``` - Transferring tokens by `amountAdd0`. ```solidity=228 uint256 refund0 = amountAdd0 - amount0; ``` - The `amountAdd0` appears to be used for bookkeeping purposes without compensating the potential transfer fees. --- ```solidity=203 IERC20(token1).safeTransferFrom(sender, address(this), amountAdd1); ``` - Transferring tokens by `amountAdd1`. ```solidity=233 uint256 refund1 = amountAdd1 - amount1; ``` - The `amountAdd1` appears to be used for bookkeeping purposes without compensating the potential transfer fees. --- ```solidity=264 _increaseLiquidityCurrentRange( tokenId, amountAdd0, amountAdd1, amount0Min, amount1Min ); ``` - Transferring tokens by `amountAdd0`. - This function call executes the following operation. - In `NTokenUniswapV3._increaseLiquidityCurrentRange`, - `IERC20(token0).safeTransferFrom(sender, address(this), amountAdd0);` ```solidity=264 _increaseLiquidityCurrentRange( tokenId, amountAdd0, amountAdd1, amount0Min, amount1Min ); ``` - This function call executes the following operation. - In `NTokenUniswapV3._increaseLiquidityCurrentRange`, - `uint256 refund0 = amountAdd0 - amount0;` - The `amountAdd0` appears to be used for bookkeeping purposes without compensating the potential transfer fees.

```solidity=279 if (reserve.xTokenAddress == address(0)) { address underlyingAsset = INToken(token) .UNDERLYING_ASSET_ADDRESS(); reserve = reservesData[underlyingAsset]; bool isNToken = reserve.xTokenAddress == token; require(isNToken, Errors.ASSET_NOT_LISTED); if (!userConfig.isUsingAsCollateral(reserve.id)) { userConfig.setUsingAsCollateral(reserve.id, true); } // No need to supply anymore because it's already NToken continue; } ``` According to the `if` condition `reserve.xTokenAddress == address(0)` of the above statement from method `_repay()`, this means that the purchased token is an `NToken`. However, this token will be locked in the `pool` of the contract forever. This may be incorrect.

```solidity=424 // Pool.sol function acceptBidWithCredit( bytes32 marketplaceId, bytes calldata payload, DataTypes.Credit calldata credit, address onBehalfOf, uint16 referralCode ) external virtual override nonReentrant { ``` Currently, the functions `buyWithCredit()`, `batchBuyWithCredit()`, `acceptBidWithCredit()`, and `batchAcceptBidWithCredit()` can be called by anyone and there is no authorization between caller and `onBehalfOf`. In our opinion, these functions would be invoked in relevant gateway contracts. - The functions `buyWithCredit()` and `batchBuyWithCredit()` could be invoked in `WETHGateway.sol`. - The functions `acceptBidWithCredit()` and `batchAcceptBidWithCredit()` could be invoked in `WPunkGateway.sol`. ```solidity // WPunkGateway.sol function acceptBidWithCredit( bytes32 marketplaceId, bytes calldata payload, DataTypes.Credit calldata credit, uint256[] calldata punkIndexes, uint16 referralCode ) external nonReentrant { ...... Pool.acceptBidWithCredit( marketplaceId, payload, credit, msg.sender, referralCode ); } ```

Referring to the comments, the logic may occur in a normal supplyERC721 pool transaction. However, when the operator is `POOL` the transaction will be reverted. That means the `UniswapV3` tokens cannot be supplied in this protocol. ```solidity=47 // if the operator is the pool, this means that the pool is transferring the token to this contract // which can happen during a normal supplyERC721 pool tx if (operator == address(POOL)) { revert(Errors.OPERATION_NOT_SUPPORTED); } ```

```solidity=149 function supplyERC721FromNToken( address asset, DataTypes.ERC721SupplyParams[] calldata tokenData, address onBehalfOf ) external virtual override nonReentrant { SupplyLogic.executeSupplyERC721FromNToken( _reserves, _usersConfig[onBehalfOf], DataTypes.ExecuteSupplyERC721Params({ asset: asset, tokenData: tokenData, onBehalfOf: onBehalfOf, actualSpender: address(0), referralCode: 0 }) ); } ``` In the function `Pool.supplyERC721FromNToken()`, any user can call it to supply ERC721 from other accounts without restriction.

Addresses should be checked before assignment or external call to make sure they are not zero addresses. --- ```solidity=75 (bool success, ) = newImplementation.delegatecall(data); ``` - `newImplementation` is not zero-checked before being used. --- ```solidity=75 _treasury = treasury; ``` - `treasury` is not zero-checked before being used. --- ```solidity=76 _underlyingAsset = underlyingAsset; ``` - `underlyingAsset` is not zero-checked before being used. --- ```solidity=75 _treasury = treasury; ``` - `treasury` is not zero-checked before being used.