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

[H-1] Bad Data Structure Choice Leads To Missing Record And Interrupted Service

zebra

Summary

Non fungible tokens from same ERC721 contract uses difference token IDs.

The nftToErc20Info mapping won't be able to store multiple ERC20Info records for the NFTs from same ERC721 contract.

mapping(address nft => ERC20Info) nftToErc20Info;

Vulnerability Details

Add following test case to TokenDividerTest.t.sol:

function testDivideNftOverwrteRecord() public {
erc721Mock.mint(USER);
vm.startPrank(USER);
uint256 TOKEN_ID_2 = 1;
// two token ids from same nft contract
erc721Mock.approve(address(tokenDivider), TOKEN_ID);
erc721Mock.approve(address(tokenDivider), TOKEN_ID_2);
​
tokenDivider.divideNft(address(erc721Mock), TOKEN_ID, AMOUNT);
TokenDivider.ERC20Info memory info = tokenDivider.getErc20InfoFromNft(address(erc721Mock));
ERC20Mock erc20ForToken1 = ERC20Mock(info.erc20Address);
​
tokenDivider.divideNft(address(erc721Mock), TOKEN_ID_2, AMOUNT);
info = tokenDivider.getErc20InfoFromNft(address(erc721Mock));
​
erc20ForToken1.approve(address(tokenDivider), AMOUNT);
// User is unable to claim TOKEN_ID because the `claimNft` function
// is checking `erc20ForToken2` instead of `erc20ForToken1`.
tokenDivider.claimNft(address(erc721Mock));
​
vm.stopPrank();
}

Impact

  1. For example, token1 and token2 are from same ERC721 contract. After

divideNft(nftAddress, token1, amount);
divideNft(nftAddress, token2, amount);

nftToErc20Info[nftAddress] will only return the ERC20Info for token2.

After that, token1 is "locked". following functions will fail:claimNft, transferErcTokens, sellErc20 and buyErc20.

  1. nftToErc20Info is the only place recording the NFT token IDs in the TokenDivider contract. Once its overwrite, the only way to found it tracking the chains.

Tools Used

foundry

Recommendations

Refactor the nftToErc20Info mapping to support different token IDs from same ERC721 contract. And update related code.

- mapping(address nft => ERC20Info) nftToErc20Info;
+ mapping(address nft => mapping(uint256 tokenId => address erc20Address)) nftToErc20Info;
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.