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

NFT Loss Due to State Variable Overwrite in divideNft Function

0x180db

Summary

The divideNft function in TokenDivider contract overwrites the nftToErc20Info mapping for each NFT address without checking if an ERC20 token already exists for that NFT, potentially causing users to lose access to their NFTs.

Vulnerability Details

In the divideNft function, the nftToErc20Info mapping is overwritten without any checks:

nftToErc20Info[nftAddress] = ERC20Info({erc20Address: erc20, tokenId: tokenId});

If a user calls divideNft multiple times for the same NFT address with different token IDs, the previous ERC20 token information will be lost, and users holding fractions of the previous ERC20 token will be unable to claim their NFT.
Proof-Of-Concept:

function setUp() public {
...
erc721Mock.mint(USER);
erc721Mock.mint(USER2);
...
}
​
function testPocLoseNft() public {
vm.startPrank(USER);
erc721Mock.approve(address(tokenDivider), 0);
tokenDivider.divideNft(address(erc721Mock), 0, AMOUNT);
vm.stopPrank();
​
vm.startPrank(USER2);
erc721Mock.approve(address(tokenDivider), 1);
tokenDivider.divideNft(address(erc721Mock), 1, AMOUNT);
vm.stopPrank();
​
ERC20Mock erc20Mock = ERC20Mock(tokenDivider.getErc20InfoFromNft(address(erc721Mock)).erc20Address);
vm.startPrank(USER);
erc20Mock.approve(address(tokenDivider), AMOUNT);
vm.expectRevert(TokenDivider.TokenDivider__NotEnoughErc20Balance.selector);
tokenDivider.claimNft(address(erc721Mock));
vm.stopPrank();
}

Impact

  • Users holding fractions of the original ERC20 token will lose their ability to claim the NFT

  • Loss of funds as the original ERC20 tokens become worthless

Tools Used

  • Manual code review

  • Performing formal verification with Quint

Recommendations

  1. Modify the mapping to track both NFT address and token ID:

mapping(address nftAddress => mapping(uint256 tokenId => ERC20Info)) nftToErc20Info;

  1. Add a check to prevent multiple divisions of the same NFT:

function divideNft(address nftAddress, uint256 tokenId, uint256 amount) external {
require(nftToErc20Info[nftAddress][tokenId].erc20Address == address(0), "NFT already divided");
// ... rest of the function
}
Updates

Lead Judging Commences

fishy Lead Judge 7 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.