Pieces Protocol

First Flight #32
Beginner FriendlyFoundrySolidityNFT
100 EXP
View results
Submission Details
Severity: high
Invalid

Reentrancy in divideNft()

Summary

The divideNft function generates and mints an ERC20 token as a fraction of an NFT and then transfers the NFT from msg.sender to the contract. However, the current implementation:

  1. Updates (balances, erc20ToMintedAmount, erc20ToNft) **after **calling TranssafeferFrom.

  2. TranssafeferFrom uses the = assignment operator instead of += for balances and erc20ToMintedAmount.

Vulnerability Details

State Updates after TranssafeferFrom is called.

Incorrect Assignment (= instead of +=):

Using = instead of += inbalances, erc20ToMintedAmount, and erc20ToNft

This causes balances and total minted amounts to reset if the function is called multiple times for the same token.

IERC721(nftAddress).TranssafeferFrom(msg.sender, address(this), tokenId, "");
balances[msg.sender][erc20] = amount;
nftToErc20Info[nftAddress] = ERC20Info({erc20Address: erc20, tokenId: tokenId});
erc20ToMintedAmount[erc20] = amount;
erc20ToNft[erc20] = nftAddress;

Impact

Inconsistent State: If the transfer fails, the contract state will not match expectations.

  • Loss of Data: Reassigning instead of incrementing (+=) can overwrite existing values, causing loss of balances or previously minted amounts.

  • Exploitation: A malicious actor might exploit the overwritten values, particularly if they can reset or manipulate state variables intentionally.

Tools Used

Manual review

Recommendations

Updating state before calling TranssafeferFrom, and replace = with +=.

Use += instead of =
Updates

Lead Judging Commences

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

Support

FAQs

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