NFT Ownership Ambiguity During Blockchain Hard Forks

[hats-bug-reporter[bot]]

Github username: -- Twitter username: -- Submission hash (on-chain): 0x005adf9a22ec6709a79a541f3af7d1a7e4664bba69ef37758614915f22d1f5d7 Severity: low

Description:

Description

The current implementation of the ERC721 token contract does not include any mechanism to distinguish between different blockchain networks in the event of a hard fork. This omission could lead to ownership ambiguity and potential disputes if the blockchain undergoes a hard fork, as the same NFT would exist on both chains with identical token IDs and metadata.

In the event of a blockchain hard fork, an attacker could exploit the situation as follows:

1. The blockchain undergoes a hard fork, creating two separate chains.
2. The NFT exists on both chains with the same token ID and metadata.
3. The attacker sells the NFT on one chain.
4. The attacker then claims ownership and sells the same NFT on the other chain.
5. This results in two different parties believing they own the "original" NFT, leading to disputes and potential loss of value for the buyers.

Proof of Concept

function tokenURI(uint256 tokenId) public view virtual returns (string memory) {
    _requireOwned(tokenId);

    string memory baseURI = _baseURI();
    return bytes(baseURI).length > 0 ? string.concat(baseURI, tokenId.toString()) : "";
}

This function, as currently implemented, does not include any chain-specific information in the token URI. This means that the same NFT would have identical metadata on both chains after a hard fork.

Revised Code Suggestion

- function tokenURI(uint256 tokenId) public view virtual returns (string memory) {
+ function tokenURI(uint256 tokenId) public view virtual returns (string memory) {
     _requireOwned(tokenId);

     string memory baseURI = _baseURI();
-    return bytes(baseURI).length > 0 ? string.concat(baseURI, tokenId.toString()) : "";
+    // Include chain ID in the token URI to differentiate between forked chains
+    return bytes(baseURI).length > 0 ? string.concat(baseURI, tokenId.toString(), "_chain", Strings.toString(block.chainid)) : "";
 }

+ // Add a function to verify the correct chain
+ function verifyChain() public view {
+     require(block.chainid == 1, "This contract is only valid on the main Ethereum chain");
+ }

1. Modified the tokenURI function to include the chain ID in the token's URI. This ensures that even if the NFT exists on multiple chains after a fork, each instance will have a unique identifier.
2. We've added a verifyChain function that can be called to ensure the contract is being interacted with on the intended chain. This function can be used as a modifier for critical operations if needed.

These changes address the vulnerability by:

1. Making each NFT uniquely identifiable across different chains.
2. Providing a mechanism to verify the correct chain, which can be used to prevent unintended interactions on forked chains.

[0xRizwan]

The said scenario is not applicable here.

When the user request withdrawal, a withdrawalId i.e an NFT would be minted to user's recipient address via requestWithdrawal() function. To claim the native ROSE token, the owner of withdrawalId can call claimWithdrawal() function which will burn the NFT in order to transfer the native ROSE tokens.