Pieces Protocol
First Flight #32
Beginner FriendlyFoundrySolidityNFT
100 EXP
View results
Previous Next
Submission Details
Severity: high
Valid

Wrong Assumption/Logical flaw in function claimNft(address nftAddress) of TokenDivider.sol

oldmonk0003

Summary

claimNft function takes address of NFT (ERC721) and transfer token id to user. It assumes that user will have only one NFT token. What if user has erc20 tokens corresponding to different token id in the NFT, which token id will the contract return? There should be token id as input the contract.

Vulnerability Details

  • Suppose Alice has minted 2 NFTs on same ERC721 contract.

  • Alice converts 2 NFTs into erc20 one after another using divideNft function of TokenDivider.sol

  • Alice wants to claim 1st NFT token back. How can she do it using claimNft? The function takes only nftAddress and nothing else as input. How will it differentiate between 2 token ids from same ERC721 contract.

/**
*
* @param nftAddress The address of the nft to claim
*
* @dev in this function, if you have all the erc20 minted for the nft, you can call this function to claim the nft,
* giving to the contract all the erc20 and it will give you back the nft
*/
​
function claimNft(address nftAddress) external {
if (nftAddress == address(0)) {
revert TokenDivider__NftAddressIsZero();
}
​
ERC20Info storage tokenInfo = nftToErc20Info[nftAddress];

Impact

User's NFT token will get locked.

Bad user experience.

Flaw in the functionality.

Tools Used

Foundry, Manual Instpection

Recommendations

Review the logic of claimNft. Consider adding tokenId as input along with nftAddress.

Updates

Lead Judging Commences

fishy Lead Judge 5 months ago
Submission Judgement Published
Validated
Assigned finding tags:

Wrong nft collection handling

Support

FAQs

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