OwnershipNFTs do not comply with ERC721, breaking composability

[c4-bot-2]

Lines of code

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

Vulnerability details

OwnershipNFTs within the contract deviate from the ERC721 standard, leading to issues with composability and interoperability. Following are the missing things which are mandated by the ERC721 specification:

• transfer() Function: The ERC721 standard specifies that a transfer to address(0) must revert, as it would destroy the token and lead to issues with tracking asset ownership. Failing to handle this can break the lifecycle of the NFT and introduce bugs or loss of tokens.

• Missing Events: Transfer and approval events are critical for tracking changes in ownership and enabling third-party systems to respond to changes in asset state.

• balanceOf() Function: This function should revert if queried for an invalid owner address (address(0)).

• ownerOf() Function: This function should revert if called with a token ID not owned by any address or if the owner is address(0).

• getApproved() Function: The function should throw an error if the queried _tokenId is not valid (i.e., the token does not exist).

Impact

The non-compliance with ERC721 results in Breaking Composability and also

• Potential Loss of Funds: Since tokens can be transferred to address(0) without reverts, this could result in permanent loss of NFTs. (This could be user error but here it is protocol mistake for not adhering the standard).

• Lack of Traceability: Missing events make it difficult to track token transfers and approvals, which can hamper monitoring systems monitoring malicious activities.

Proof of Concept

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

Tools Used

Manual Review

Recommended Mitigation Steps

Align OwnershipNFT contract with the ERC721 standard.

Assessed type

ERC721

[af-afk]

fluidity-money/long.so@cb98ea6