Incorrect verification of onERC721Received callback success

[c4-bot-5]

Lines of code

https://github.com/code-423n4/2024-08-superposition/blob/main/pkg/sol/OwnershipNFTs.sol#L92-L95

Vulnerability details

Impact

Incorrect verification of onERC721Received callback can lead to the inability to transfer OwnershipNFTs to contracts.

Proof of Concept

According to the EIP-721 specification, the OwnershipNFTs.sol contract checks whether the recipient is a contract. If so, it validates the success of the transfer inside the _onTransferReceived function:

    function _onTransferReceived(
        address _sender,
        address _from,
        address _to,
        uint256 _tokenId
    ) internal {
        // only call the callback if the receiver is a contract
        if (_to.code.length == 0) return;

        bytes4 data = IERC721TokenReceiver(_to).onERC721Received(
            _sender,
            _from,
            _tokenId,

            // this is empty byte data that can be optionally passed to
            // the contract we're confirming is able to receive NFTs
            ""
        );

>>      require(
            data != IERC721TokenReceiver.onERC721Received.selector,
            "bad nft transfer received data"
        );
    }

However, instead of checking that the returned data equals the onERC721Received.selector "magic word", the contract requires the opposite. This logic error causes NFT transfers to contract addresses to revert even when they are successful.

Tools Used

Manual review

Recommended Mitigation Steps

        require(
+           data == IERC721TokenReceiver.onERC721Received.selector,
            "bad nft transfer received data"
        );

Assessed type

ERC721

[c4-judge]

alex-ppg marked the issue as satisfactory