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

Inadequate `msg.value` Check Allows NFTs to be Locked with Minimal Payment

bbash

Summary

In the trickOrTreat function of the SpookySwap contract, a user can exploit the lack of an initial msg.value check by sending a minimal amount of ETH (e.g., 1 wei) for multiple calls. When the random condition for a "trick" (double price) is met, the NFT is minted to the contract with 1 wei as pendingNFTsAmountPaid. Since there is no requirement to call resolveTrick, users may abandon the NFT purchase process, leaving the NFT locked in the contract without adequate compensation. This can lead to storage bloat and create a DoS risk by filling the contract’s storage with orphaned NFTs, ultimately affecting contract functionality and user experience.

Vulnerability Details

The issue can be found here at : https://github.com/Cyfrin/2024-10-trick-or-treat/blob/main/src/TrickOrTreat.sol#L48-L105

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;
​
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
if (msg.value > requiredCost) {
uint256 refund = msg.value - requiredCost;
(bool refundSuccess,) = msg.sender.call{value: refund}("");
require(refundSuccess, "Refund failed");
}
}

Impact

Continuous exploitation may cause the contract to store a large number of abandoned NFTs, which could eventually exhaust storage capacity, causing the contract to reach operational limits or even face high storage costs. The accumulation of orphaned NFTs makes it harder for legitimate users to mint new NFTs or engage with the contract, reducing the contract’s efficiency and reliability. The contract owner may incur costs due to unclaimed NFTs in storage without adequate payment, leading to a financial loss over time.

Tools Used

Manual Review

Recommendations

To address this issue, introduce an initial msg.value check in the trickOrTreat function to ensure the user has sent at least the base treat.cost. This ensures a minimum commitment, deterring abuse from minimal payments.

require(treat.cost > 0, "Treat cost not set.");
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.