Upgraded Q -> M from #72 [1674644492627]

[c4-judge]

Judge has assessed an item in Issue #72 as M risk. The relevant finding follows:

[Low-03] NTokenMoonBirds may not be able to receive airdrops Impact For most NToken, some airdrops that are actively minted to the holder's address can be withdrawn and later distributed by the PoolAdmin calling the rescueERC721 function.

function rescueERC721(
    address token,
    address to,
    uint256[] calldata ids
) external override onlyPoolAdmin {
    require(
        token != _underlyingAsset,
        Errors.UNDERLYING_ASSET_CAN_NOT_BE_TRANSFERRED
    );
    for (uint256 i = 0; i < ids.length; i++) {
        IERC721(token).safeTransferFrom(address(this), to, ids[i]);
    }
    emit RescueERC721(token, to, ids);
}

However, in the onERC721Received function of the NTokenMoonBirds contract, due to the requirement that the sender can only be the MoonBird contract, when safemint()/safetransferfrom() is called to send the airdrop NFTs to the NTokenMoonBirds contract, the transaction will fail, thus preventing NTokenMoonBirds from receiving these airdrops.

function onERC721Received(
    address operator,
    address from,
    uint256 id,
    bytes memory
) external virtual override returns (bytes4) {
    // only accept MoonBird tokens
    require(msg.sender == _underlyingAsset, Errors.OPERATION_NOT_SUPPORTED);

For example, Moonbirds Oddities are actively minted to the holder's address. https://etherscan.io/tx/0x3af5de8b6a8c55aac033d57e1b110e8340abf4dcd289ebda889a44f9f9dc613d

Proof of Concept https://github.com/code-423n4/2022-11-paraspace/blob/c6820a279c64a299a783955749fdc977de8f0449/paraspace-core/contracts/protocol/tokenization/NToken.sol#L136-L149 https://github.com/code-423n4/2022-11-paraspace/blob/c6820a279c64a299a783955749fdc977de8f0449/paraspace-core/contracts/protocol/tokenization/NTokenMoonBirds.sol#L63-L77

Recommended Mitigation Steps Consider allowing the NTokenMoonBirds contract to receive NFTs from other addresses and only call POOL.supportERC721FromNToken when msg.sender == _underlyingAsset

function onERC721Received(
    address operator,
    address from,
    uint256 id,
    bytes memory
) external virtual override returns (bytes4) {
    // only accept MoonBird tokens

• require(msg.sender == _underlyingAsset, Errors.OPERATION_NOT_SUPPORTED);

  // 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)) {
      return this.onERC721Received.selector;
  }

• if(msg.sender == _underlyingAsset){
  // supply the received token to the pool and set it as collateral DataTypes.ERC721SupplyParams[] memory tokenData = new DataTypes.ERC721SupplyParams;

  tokenData[0] = DataTypes.ERC721SupplyParams({
      tokenId: id,
      useAsCollateral: true
  });
  
  POOL.supplyERC721FromNToken(_underlyingAsset, tokenData, from);

• } return this.onERC721Received.selector; }

[c4-judge]

dmvt marked the issue as duplicate of #164

[c4-judge]

dmvt marked the issue as satisfactory