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

Irreversible ERC20 Contract Creation Prior to NFT Transfer Verification

teoslaf

Summary

In the current implementation of the divideNft function, the ERC20 contract is created and tokens are minted before verifying whether the NFT transfer to the contract succeeds. This order of operations creates an issue because if the NFT transfer fails, the transaction reverts, undoing state changes in the main contract, but the creation of the ERC20 contract cannot be undone.

Vulnerability Details

The current code creates and mints the ERC20 contract before transferring the NFT:

ERC20ToGenerateNftFraccion erc20Contract = new ERC20ToGenerateNftFraccion(...);
erc20Contract.mint(address(this), amount);
address erc20 = address(erc20Contract);
​
IERC721(nftAddress).safeTransferFrom(msg.sender, address(this), tokenId, "");
if(IERC721(nftAddress).ownerOf(tokenId) == msg.sender) {
revert TokenDivider__NftTransferFailed();
}

If the NFT transfer fails, the revert statement undoes the state changes in the divideNft function, but the ERC20 contract remains deployed on the blockchain, leading to potential unintended consequences:

  • Wasted gas for the user.

  • Accumulation of unused or orphaned ERC20 contracts.

  • Increased difficulty in managing or cleaning up the blockchain state.

Impact

This issue can lead to inefficiencies, confusion for users, and potential vulnerabilities stemming from unintended ERC20 contract deployments.

Tools Used

Manual code review.

Recommendations

To prevent this issue, reorder the function so the NFT transfer is completed and verified before creating the ERC20 contract:

function divideNft(address nftAddress, uint256 tokenId, uint256 amount) external {
// Checks
if(nftAddress == address(0)) { revert TokenDivider__NftAddressIsZero(); }
if(amount == 0) { revert TokenDivider__AmountCantBeZero(); }
​
// Transfer NFT first
IERC721(nftAddress).safeTransferFrom(msg.sender, address(this), tokenId, "");
if(IERC721(nftAddress).ownerOf(tokenId) != address(this)) {
revert TokenDivider__NftTransferFailed();
}
​
// Only create tokens after NFT transfer succeeds
ERC20ToGenerateNftFraccion erc20Contract = new ERC20ToGenerateNftFraccion(...);
erc20Contract.mint(address(this), amount);
address erc20 = address(erc20Contract);
​
// Rest of the function
...
}

This approach ensures that the ERC20 contract is only created if the NFT transfer succeeds, preventing unnecessary gas expenditure and the creation of orphaned ERC20 contracts.

Updates

Lead Judging Commences

fishy Lead Judge 5 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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