Submitted by bart1e, also found by rokinot, gkrastenov, HHK, bin2chen, jasonxiale, Aymen0909, josephdara, pep7siup, peakbolt, Inspex, kodyvim, tapir, 0x3b, 0xCiphky, rvierdiiev, and chaduke
https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/amo/UniV3LiquidityAmo.sol#L324-L334 
https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/core/RdpxV2Core.sol#L1-L1308
UniV3LiquidityAMO::recoverERC721 is a function created in order to be able to recover ERC721 tokens from the UniV3LiquidityAMO contract. It can only be called by admin and will transfer all ERC721 tokens to the RdpxV2Core contract. The problem is, that it wont be possible to do anything with these tokens after they are transferred to rdpxV2Core.
Indeed, RdpxV2Core inherits from the following contracts:
and no contract from this list implement any logic allowing the NFT transfer (only ERC721Holder has something to do with NFTs, but it only allows to receive them, not to approve or transfer).
Moreover, rdpxV2Core also doesnt have any logic allowing transfer or approval of NFTs:
UniV3LiquidityAMO::recoverERC721 instead of recovering ERC721, locks all tokens in rdpxV2Core and it wont be possible to recover them from that contract.
Any use of recoverERC721 will imply an irrecoverable loss for the protocol and this function was implemented in order to be used at some point after all (even if only on emergency situations). Because of that, Im submitting this issue as High.
This PoC only shows that ERC721 token recovery will not be possible by calling RdpxV2Core::approveContractToSpend. Lack of functions doing transfer or approve or any other ERC721 related functions in RdpxV2Core may just be observed by looking at the contracts code.
Please create the MockERC721.sol file in mocks directory and with the following code:
It will just mint an ERC721 token with tokenId = 1.
Please also run the following test:
VS Code
Either implement additional ERC721 recovery function in RdpxV2Core or change UniV3LiquidityAMO::recoverERC721 so that it transfers all NFTs to msg.sender instead of RdpxV2Core contract.
psytama (Dopex) confirmed and commented:
Alex the Entreprenerd (Judge) commented:
