Trick or Treat

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

Incorrect Event Emission for Insufficient Payment in NFT Minting

01. Relevant GitHub Links

02. Summary

The contract emits a Swapped event even when a user has not sent enough ETH to complete the transaction. In such cases, the NFT is minted to the contract and marked as pending, rather than being directly swapped. This incorrect event can cause confusion by suggesting the transaction succeeded and the NFT was swapped to the user, while it’s actually still pending.

03. Vulnerability Details

When a user sends insufficient ETH, the contract mints an NFT to itself and marks it as pending by recording the user’s address and the amount paid. However, the contract still emits the Swapped event, which falsely signals that the transaction was successful and that the NFT was swapped to the user. This misrepresentation can lead to user confusion or misunderstandings about the transaction status.

} else {
// User didn't send enough ETH
// Mint NFT to contract and store pending purchase
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 Swapped(msg.sender, _treatName, tokenId);

This event should only be emitted if the transaction completes successfully and the NFT is actually transferred to the user, not when it’s stored as a pending NFT.

03. Impact

  • User Confusion and Misinterpretation: The Swapped event indicates a successful transaction, which can mislead users into thinking their purchase was completed when, in reality, the NFT is still pending.

  • Potential Application Logic Errors: External systems or interfaces relying on events for status updates may misinterpret the transaction status, leading to potential errors in application logic or user interfaces.

04. Proof of Concept

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.24;
import {Test, console} from "forge-std/Test.sol";
import {SpookySwap} from "../src/TrickOrTreat.sol";
contract TrickOrTreatTest is Test {
SpookySwap spookyswapInstance;
address owner;
address user;
function setUp() public {
owner = vm.addr(0x1);
user = vm.addr(0x2);
vm.label(owner, "Owner");
vm.deal(owner, 100 ether);
vm.label(user, "User");
vm.deal(user, 100 ether);
SpookySwap.Treat[] memory TreatList;
vm.prank(owner);
spookyswapInstance = new SpookySwap(TreatList);
}
function test_IncorrectEvent() public {
vm.prank(owner);
spookyswapInstance.addTreat("Example", 1 ether, "test");
uint256 tokenId = spookyswapInstance.nextTokenId();
// random = 2
// double price
vm.warp(123456790);
vm.prevrandao(987654324);
vm.expectEmit(true, true, true, true);
// Emits an event indicating the treat was swapped to the user
emit SpookySwap.Swapped(user, "Example", tokenId);
vm.prank(user);
// Insufficient funds: treat purchase is pending until full payment is made
spookyswapInstance.trickOrTreat{value: 1 ether}("Example");
// user have no nft
vm.assertEq(0, spookyswapInstance.balanceOf(user));
}
}

05. Tools Used

Manual Code Review and Foundry

06. Recommended Mitigation

  1. Emit a Different Event for Pending Transactions: Introduce a new event, such as PendingSwap, specifically for cases where the NFT is stored in the contract due to insufficient payment. This provides clarity on the transaction’s actual status.

emit PendingSwap(msg.sender, _treatName, tokenId);
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.