Trick or Treat
First Flight #27
Beginner FriendlyFoundry
100 EXP
View results
Previous Next
Submission Details
Severity: low
Invalid

If a user triggers the "double price" scenario (random == 2) in `SpookySwap::trickOrTreat` but never calls `SpookySwap::resolveTrick`, the NFT remains locked in the contract. This creates a DOS risk for other users, preventing the reuse

xilobytes

Description

In the case a user that calls SpookySwap::trickOrTreat and hits the scenario where random variable gets the value of 2, the NFT is minted to the SpookySwap contract instead to the user. After that, the user has to call SpookySwap::resolveTrick in order to pay for the remainding amount, and finally receive the NFT. But if the user never calls the SpookySwap::resolveTrick to complete the pending purchase, the NFT will stay in the contract.

Impact

Pending Treats that are not resolved will remain stuck in the contract, preventing the option for other users to mint that specific tokenId. Also, the user will not be able to get a refund of the initial ETH amount sent in case the contract owner decides to do something with the NFT.

Proof of Concepts

  1. The user calls SpookySwap::trickOrTreat with less than the double of the price in ETH

  2. The user never calls SpookySwap::resolveTrick to complete the pending purchase.

  3. Due to the requirement require(pendingNFTs[tokenId] == msg.sender, "Not authorized to complete purchase"); no other user would be able to complete the purchase.

Tools Used
Foundry, VsCode

Recommended mitigation

Set a timeframe in which users have to resolve the trick, if they fail to do so, refund the amount of ETH they originally sent.

Add the following code in SpookySwap contract, inside SpookySwap::trickOrTreat, and SpookySwap::resolveTrick respectively.

​
contract SpookySwap is ERC721URIStorage, Ownable(msg.sender), ReentrancyGuard {
​
+ mapping(uint256 => uint256) public pendingNFTsTimestamp; // tokenId => timestamp of the pending purchase getting initialized
+ uint256 public constant TRICK_RESOLUTION_PERIOD = 7 days; // Example of a 7-day timeframe
​
***
​
​
***
​
function trickOrTreat(string memory _treatName) public payable nonReentrant {
Treat memory treat = treatList[_treatName];
require(treat.cost > 0, "Treat cost not set.");
​
uint256 costMultiplierNumerator = 1;
uint256 costMultiplierDenominator = 1;
​
// Generate a pseudo-random number between 1 and 1000
uint256 random =
uint256(keccak256(abi.encodePacked(block.timestamp, msg.sender, nextTokenId, block.prevrandao))) % 1000 + 1;
​
if (random == 1) {
// 1/1000 chance of half price (treat)
costMultiplierNumerator = 1;
costMultiplierDenominator = 2;
} else if (random == 2) {
// 1/1000 chance of double price (trick)
costMultiplierNumerator = 2;
costMultiplierDenominator = 1;
}
// Else, normal price (multiplier remains 1/1)
uint256 requiredCost = (treat.cost * costMultiplierNumerator) / costMultiplierDenominator;
​
if (costMultiplierNumerator == 2 && costMultiplierDenominator == 1) {
// Double price case (trick)
if (msg.value >= requiredCost) {
// User sent enough ETH
mintTreat(msg.sender, treat);
} 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;
+ pendingNFTsTimestamp[tokenId] = block.timestamp;
​
emit Swapped(msg.sender, _treatName, tokenId);
​
// User needs to call fellForTrick() to finish the transaction
}
} else {
// Normal price or half price
require(msg.value >= requiredCost, "Insufficient ETH sent for treat");
mintTreat(msg.sender, treat);
}
​
// Refund excess ETH if any
// q does something happen if msg.sender is not set as payable?
if (msg.value > requiredCost) {
uint256 refund = msg.value - requiredCost;
(bool refundSuccess,) = msg.sender.call{value: refund}("");
require(refundSuccess, "Refund failed");
}
}
​
***
​
​
***
​
function resolveTrick(uint256 tokenId) public payable nonReentrant {
require(pendingNFTs[tokenId] == msg.sender, "Not authorized to complete purchase");
+ require(pendingNFTsTimestamp[tokenId] > 0, "NFT not pending");
​
string memory treatName = tokenIdToTreatName[tokenId];
Treat memory treat = treatList[treatName];
​
uint256 requiredCost = treat.cost * 2; // Double price
uint256 amountPaid = pendingNFTsAmountPaid[tokenId];
uint256 totalPaid = amountPaid + msg.value;
​
+ if (block.timestamp >= pendingNFTsTimestamp[tokenId] + TRICK_RESOLUTION_PERIOD) {
+ // Refund the user if the resolution period expired
+ delete pendingNFTs[tokenId];
+ delete pendingNFTsAmountPaid[tokenId];
+ delete tokenIdToTreatName[tokenId];
​
+ (bool success, ) = payable(msg.sender).call{value: amountPaid}("");
+ require(success, "Refund failed");
​
+ } else {
require(totalPaid >= requiredCost, "Insufficient ETH sent to complete purchase");
​
// Transfer the NFT to the buyer
_transfer(address(this), msg.sender, tokenId);
​
delete pendingNFTs[tokenId];
delete pendingNFTsAmountPaid[tokenId];
delete tokenIdToTreatName[tokenId];
​
// Refund excess ETH if any
if (totalPaid > requiredCost) {
uint256 refund = totalPaid - requiredCost;
(bool refundSuccess,) = msg.sender.call{value: refund}("");
require(refundSuccess, "Refund failed");
}
}
}
}
Updates

Appeal created

bube Lead Judge 10 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

[invalid] Unlimited pending NFTs

The protocol can work correctly with more than 20000 tokens in it. It is informational.

Support

FAQs

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