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

Token Burn Before NFT Transfer Causes Permanent Loss

udogodwin2k22

Summary

The claimNft() function in the TokenDivider contract contains a critical vulnerability where ERC20 tokens are burned before confirming the NFT transfer. This sequencing error can lead to a permanent loss of user tokens if the NFT transfer fails due to various reasons, leaving users with no compensation for their burned tokens.

Vulnerability Details:

https://github.com/Cyfrin/2025-01-pieces-protocol/blob/4ef5e96fced27334f2a62e388a8a377f97a7f8cb/src/TokenDivider.sol#L147-L167

in the Current Vulnerable Implementation, according to the order of operations in the claimNft() function. Tokens are burned immediately without ensuring the NFT transfer succeeds, creating a risk of permanent loss in scenarios where the transfer encounters unexpected failures.

function claimNft(address nftAddress) external {
// Burn tokens BEFORE NFT transfer
ERC20ToGenerateNftFraccion(tokenInfo.erc20Address).burnFrom(
msg.sender,
erc20ToMintedAmount[tokenInfo.erc20Address]
);
​
// Critical vulnerability: Tokens burned before transfer
IERC721(nftAddress).safeTransferFrom(address(this), msg.sender, tokenInfo.tokenId);
}

The burn operation occurs before confirming the successful NFT transfer, Network or storage issues could cause the transfer to fail unexpectedly and there is no mechanism to revert the token burn if the NFT transfer fails.

Using real world scenario case,

  1. Alice owns all fractional tokens for an NFT

  2. She attempts to claim the full NFT

  3. Tokens are immediately burned

  4. NFT transfer fails due to:

    • Contract no longer owning the NFT or

    • Unexpected blockchain condition

  5. Alice permanently loses her tokens WITHOUT receiving the NFT

Impact

  • Users can lose 100% of their token value

  • Potential complete loss of fractional NFT investment

  • Significant financial risk for all platform users

Recommendation

Reordering operations within the claimNft() function can eliminate this vulnerability. The NFT transfer must occur successfully before the ERC20 tokens are burned.

function claimNft(address nftAddress) external {
if(nftAddress == address(0)) {
revert TokenDivider__NftAddressIsZero();
}
​
ERC20Info storage tokenInfo = nftToErc20Info[nftAddress];
​
// Verify sufficient balance FIRST
if(balances[msg.sender][tokenInfo.erc20Address] < erc20ToMintedAmount[tokenInfo.erc20Address]) {
revert TokenDivider__NotEnoughErc20Balance();
}
​
// Transfer NFT FIRST - Ensures ownership before burning
IERC721(nftAddress).safeTransferFrom(address(this), msg.sender, tokenInfo.tokenId);
​
// Burn tokens AFTER successful transfer
ERC20ToGenerateNftFraccion(tokenInfo.erc20Address).burnFrom(
msg.sender,
erc20ToMintedAmount[tokenInfo.erc20Address]
);
​
// Reset balances after confirmed transfer
balances[msg.sender][tokenInfo.erc20Address] = 0;
erc20ToMintedAmount[tokenInfo.erc20Address] = 0;
​
emit NftClaimed(nftAddress);
}
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.