Trick or Treat

First Flight #27
Beginner FriendlyFoundry
100 EXP
View results
Submission Details
Severity: low
Valid

Incorrect Event Emission in `trickOrTreat`

Summary

  • Root Cause: Swapped event is emitted during NFT escrow to contract instead of actual user claim

  • Impact: Off-chain systems could misinterpret NFT ownership and transaction state

Vulnerability Details

In the trick scenario where insufficient ETH is sent, the contract:

  1. Mints the NFT to itself (contract) as escrow

  2. Incorrectly emits a Swapped event showing the user as the recipient

  3. Actually transfers the NFT to user only later in resolveTrick

  4. No event is emitted when the actual transfer to user happens

Here is the current implementation:

// During trickOrTreat when price is doubled:
_mint(address(this), tokenId);
// ... storage updates ...
emit Swapped(msg.sender, _treatName, tokenId); // WRONG! NFT is not with msg.sender yet
// During resolveTrick:
_transfer(address(this), msg.sender, tokenId);
// No event emission! // WRONG! Should emit here

This creates several issues:

  1. Off-chain systems might think user owns NFT before they actually do

  2. No event is emitted when actual ownership transfer happens

  3. Initial event has incorrect ownership information

  4. Makes tracking NFT state more difficult

Impact

Off chain system will report wrong data regarding nft ownership

Tools Used

Manual Review

Recommendations

event TreatEscrowed(address indexed intendedRecipient, string treatName, uint256 tokenId, uint256 paidAmount, uint256 requiredAmount);
event TreatClaimed(address indexed recipient, string treatName, uint256 tokenId);
// In trickOrTreat:
if (msg.value < requiredCost) {
uint256 tokenId = nextTokenId;
_mint(address(this), tokenId);
_setTokenURI(tokenId, treat.metadataURI);
nextTokenId += 1;
pendingNFTs[tokenId] = msg.sender;
pendingNFTsAmountPaid[tokenId] = msg.value;
tokenIdToTreatName[tokenId] = _treatName;
emit TreatEscrowed(msg.sender, _treatName, tokenId, msg.value, requiredCost);
}
// In resolveTrick:
function resolveTrick(uint256 tokenId) public payable nonReentrant {
// ... existing checks ...
_transfer(address(this), msg.sender, tokenId);
emit TreatClaimed(msg.sender, tokenIdToTreatName[tokenId], tokenId);
// ... cleanup ...
}
Updates

Appeal created

bube Lead Judge 9 months ago
Submission Judgement Published
Validated
Assigned finding tags:

Incorrect Swapped event emission

The protocol emits a Swapped event even when the user has not sent enough ETH to complete the transaction.

Support

FAQs

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