Lines of code

https://github.com/code-423n4/2024-08-superposition/blob/4528c9d2dbe1550d2660dac903a8246076044905/pkg/sol/OwnershipNFTs.sol#L93

Vulnerability details

Impact

Function OwnershipNFT::_onTransferReceived should revert when callback does not return the selector, but it does the opposite and reverts when callback returns selector.

https://github.com/code-423n4/2024-08-superposition/blob/4528c9d2dbe1550d2660dac903a8246076044905/pkg/sol/OwnershipNFTs.sol#L93

    /**
     * @notice _onTransferReceived by calling the callback `onERC721Received`
     *         in the recipient if they have codesize > 0. if the callback
     *         doesn't return the selector, revert!
     * @param _sender that did the transfer
     * @param _from owner of the NFT that the sender is transferring
     * @param _to recipient of the NFT that we're calling the function on
     * @param _tokenId that we're transferring from our internal storage
     */
    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, //@audit wrong statement
            "bad nft transfer received data"
        );
    }

Tools Used

Manual review

Recommended Mitigation Steps

Change != into ==

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

Assessed type

DoS

code-423n4 / 2024-08-superposition-validation