DeFiFoundry
60,000 USDC
View results
Submission Details
Severity: medium
Invalid

NFT owners cannot transfer ownership

https://github.com/Cyfrin/2024-07-zaros/blob/main/src/account-nft/AccountNFT.sol#L23-L28

Summary

NFTs in AccountNFT.sol cannot be transferred because there is no external function or helper function that uses the internal _update function. This oversight prevents the transfer of NFT ownership, which is essential for maintaining the security and functionality of the protocol.

Description

In the AccountNFT.sol contract, the _update function is crucial for updating the ownership of NFT accounts. However, since this function is declared as internal, it cannot be accessed by any externally owned account (EOA) or external contract. Additionally, there is no external or public function within AccountNFT.sol that calls this internal _update function, making it impossible for users to transfer ownership of their NFT accounts. This restriction prevents the transfer of NFT account ownership, which is essential for maintaining security and functionality within the protocol. If a user's wallet is compromised, they would be unable to transfer ownership of their trading accounts and any associated positions to a new, secure wallet.

Impact

The inability to transfer ownership of NFT accounts poses a significant risk to protocol functionality. If an individual's wallet is compromised, they cannot secure their assets by transferring ownership to a new wallet. While the assets themselves may not be directly at risk, the protocol's operation is severely hampered as users lose the ability to manage their accounts effectively.

Proof of Concept

The _update function is declared as internal, making it inaccessible to external entities:

function _update(address to, uint256 tokenId, address auth) internal virtual override returns (address) {
address previousOwner = super._update(to, tokenId, auth);
IPerpsEngine(owner()).notifyAccountTransfer(to, tokenId.toUint128());
return previousOwner;
}

Since there is no public or external method within AccountNFT.sol that wraps this function, external access to this functionality is blocked, preventing the transfer of ownership of NFT accounts.

Tools Used

Manual Review

Recommended Mitigation Steps

Add External Function: Introduce an external function that wraps the _update function, allowing external accounts and contracts to access this functionality.

Updates

Lead Judging Commences

inallhonesty Lead Judge
about 1 year ago
inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.