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

Lack of Burn Verification and Insufficient Balance/Allowance Checks in claimNft Function

teoslaf

Summary

The claimNft function lacks proper checks for ERC20 token balance and allowance, and does not verify that the burn operation was successful. This can lead to inconsistent state changes and potential loss of funds if the burn operation fails.

Vulnerability Details

  1. Lack of Balance and Allowance Checks
    The function does not explicitly verify whether the user has enough balance or allowance to burn the required amount of ERC20 tokens. If the user doesn't have sufficient tokens or allowance, the transaction could fail, but this isn't properly handled, leading to potential inconsistencies.

  2. Burn Failure Not Verified
    The burn operation is executed without verifying whether it succeeded. If the burn fails or does not reduce the user's balance as expected, the state could be updated incorrectly, resulting in lost tokens or NFTs.

  3. State Updated Before Burn Success
    The state (i.e., token balances and minted amounts) is updated before confirming the burn was successful. If the burn fails, the state would not be reverted, leading to inconsistent contract state.

Impact

  • State Inconsistency: If the burn fails or the user doesn't have sufficient balance/allowance, the contract may update the state incorrectly.

  • Potential Loss of Funds: Users could lose their ERC20 tokens or NFTs if the burn operation is not properly verified, and the state updates incorrectly.

  • Misleading Behavior: The contract will emit the NftClaimed event and transfer the NFT even if the burn fails, which is misleading to the user and off-chain services.

Tools Used

Manual code review

Recommendations

  1. Check ERC20 Balance and Allowance: Explicitly verify the user's balance and allowance before attempting to burn tokens.

  2. Use Try/Catch for Burn Operation: Wrap the burn operation in a try/catch block to handle any potential errors during the burn process.

  3. Verify Burn Success: After the burn, check that the user's balance was correctly reduced to ensure the burn was successful.

  4. Update State After Successful Burn: Only update the state (balances, minted amounts) after confirming that the burn succeeded.

  5. Emit Events After Successful Operations: Only emit the NftClaimed event after the burn and state updates have been successfully confirmed.

function claimNft(address nftAddress) external {
if(nftAddress == address(0)) {
revert TokenDivider__NftAddressIsZero();
}
​
ERC20Info storage tokenInfo = nftToErc20Info[nftAddress];
uint256 amountToBurn = erc20ToMintedAmount[tokenInfo.erc20Address];
​
// Check actual balance
if(IERC20(tokenInfo.erc20Address).balanceOf(msg.sender) < amountToBurn) {
revert TokenDivider__InsufficientTokenBalance();
}
​
// Check allowance
if(IERC20(tokenInfo.erc20Address).allowance(msg.sender, address(this)) < amountToBurn) {
revert TokenDivider__InsufficientAllowance();
}
​
// Try to burn
try ERC20ToGenerateNftFraccion(tokenInfo.erc20Address).burnFrom(msg.sender, amountToBurn) {
// Verify burn success
if(IERC20(tokenInfo.erc20Address).balanceOf(msg.sender) != 0) {
revert TokenDivider__BurnFailed();
}
// Update state after successful burn
balances[msg.sender][tokenInfo.erc20Address] = 0;
erc20ToMintedAmount[tokenInfo.erc20Address] = 0;
emit NftClaimed(nftAddress);
// Transfer NFT last
IERC721(nftAddress).safeTransferFrom(address(this), msg.sender, tokenInfo.tokenId);
} catch {
revert TokenDivider__BurnFailed();
}
}
​
Updates

Lead Judging Commences

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

Support

FAQs

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