Lines of code

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

Vulnerability details

Impact

Vulnerability Details

OpenZeppelin's ERC721.sol includes callback functions to manage NFTs and prevent them from getting stuck in contracts. For NFT contracts, there exist some implicit external function calls that could be neglected by developers. They include onERC721Received function. The onERC721Received function was designed to check whether the receiver contract can handle NFTs. This function is invoked in the safeTransferFrom() of the OwnershipNFTs contract. Due to this external function call, the reentrancy could happen without being noticed by contract developer.

According to what I said above, the malicious person can abuse the same things and write in the onERC721Received function of his contract to call the safeTransferFrom() function many times in a loop in one transaction.

In the safeTransferFrom() function of the OwnershipNFTs.sol contract, the _onTransferReceived() function is called.

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

    function safeTransferFrom(
        address _from,
        address _to,
        uint256 _tokenId
    ) external payable {
        _transfer(_from, _to, _tokenId);
        _onTransferReceived(msg.sender, _from, _to, _tokenId);
    }

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

    function safeTransferFrom(
        address _from,
        address _to,
        uint256 _tokenId,
        bytes calldata /* _data */
    ) external payable {
        _transfer(_from, _to, _tokenId);
        _onTransferReceived(msg.sender, _from, _to, _tokenId);
    }

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

    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"
        );
    }

POC :

Attacker calls the safeTransferFrom() function.
Due to the lack of Reentrancy Guard adherence, the attacker re-enters the safeTransferFrom() function in onERC721Received before the initial call completes.
This process can be repeated, allowing the attacker to transfer token/NFT more than their allowed.