Submission Details
Severity:
New NFTs being divided are overwritting the old ones
Vulnerability Details
The TokenDivider::divideNft function overwrites an already existing nft in the nftToErc20Info mapping every time and there is no way to fetch these previously divided nfts.
Impact
Previously stored nfts in nftToErc20Info are overwritten and locked up in the contract, unable to be queried.
Tools Used
Manual review
Proof of Concept
USER mints nft and divides it
USER mints nft with the same address and divides it
there is no way to query the 1st nft, only the 2nd one
PoC Code
Add following test:
function testDivideMultipleNftsWithTheSameAddress() public nftDivided {
// mint, approve and divide two more nfts
erc721Mock.mint(USER);
erc721Mock.mint(USER);
vm.startPrank(USER);
erc721Mock.approve(address(tokenDivider), 1);
tokenDivider.divideNft(address(erc721Mock), 1, AMOUNT);
uint256 tokenId = tokenDivider
.getErc20InfoFromNft(address(erc721Mock))
.tokenId;
erc721Mock.approve(address(tokenDivider), 2);
tokenDivider.divideNft(address(erc721Mock), 2, AMOUNT);
vm.stopPrank();
uint256 tokenIdAfter = tokenDivider
.getErc20InfoFromNft(address(erc721Mock))
.tokenId;
// tokenIdAfter is 2 even tho tokenId is 1, but its fetching the same thing
// there is no way to distinguish between them
// the new one always overrides the old one
// I can't get nft id 0 nor 1 that came before it
console.log("tokenId", tokenId);
assertEq(tokenId, tokenIdAfter);
}
Recommendations
To prevent this, we should store each nft using a unique ID.
Updates
Lead Judging Commences
fishy 5 months ago
Submission Judgement Published Assigned finding tags:
Wrong nft collection handling
Rewards breakdown
nSLOC
181This contest has a flat reward structure with the following payouts:
100 EXP
20 EXP
2 EXP
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.