hats-finance / Convergence---Convex-integration-0xb3df23e155b74ad2b93777f58980d6727e8b40bb

0 stars 1 forks source link

NFT does not Handle Hard Forks Appropriately #20

Open hats-bug-reporter[bot] opened 6 months ago

hats-bug-reporter[bot] commented 6 months ago

Github username: @notbozho Twitter username: 0xBozho Submission hash (on-chain): 0x0bd28c251733bdf5c5dfc318aea0e09e59d21ab36bc0914862d04aea2ad45ce2 Severity: low

Description:

Description

The smart contract implementation in CvxStakingPositionManager.sol does not handle blockchain hard forks effectively, potentially leading to confusion and disputes over the ownership of NFTs across different chains. This issue stems from the contract's lack of mechanisms to acknowledge the existence of multiple chains after a hard fork, which can result in the same NFT being claimed on multiple chains without clear differentiation.

Attack Scenario

During a hard fork, the blockchain diverges into two separate chains, each with its own state but sharing history up to the fork point. NFTs minted before the fork can exist in both chains, leading to potential conflicts in ownership and usage rights. An attacker could exploit this ambiguity to assert ownership or use rights in a chain where they originally didn't hold the NFT, especially if the contract lacks explicit reference to the chain ID in its functions or metadata.

Affected Code

📁 File: contracts/Staking/Convex/CvxStakingPositionManager.sol
185:     function tokenURI(uint256 tokenId) public view virtual override returns (string memory) { 
186:         ICvxStakingLogo _logo = logo;
187: 
188:         if (address(_logo) == address(0)) {
189:             string memory uri = _baseURI();
190:             return bytes(uri).length != 0 ? string(abi.encodePacked(uri, Strings.toString(tokenId))) : "";
191:         }
192: 
193:         return _logo._tokenURI(logoInfo(tokenId));
194:     }

View on GitHub

Recommendations

PlamenTSV commented 6 months ago

In the extremely unlikely event of an unpredicted hard fork, the base URI can be managed on both chains to not produce the same value. Good recommendation, too unlikely scenario to consider valid imo.