Trick or Treat

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

The `payable` type is not implemented inside `SpookySwap::trickOrTreat` nor `SpookySwap::resolveTrick`, preventing the refund to be sent to addresses that don't support receiving Ether.

Description

In the functions SpookySwap::trickOrTreat and SpookySwap::resolveTrick, during the refund process the msg.sender is not cast as a payable address in (bool refundSuccess,) = msg.sender.call{value: refund}(""); like payable(msg.sender).

Impact

If the msg.sender address cannot receive Ether, at runtime the refund will fail because only payable addresses can receive Ether. The refund would result in refundSuccess being set to false, causing the refund to never happen and making the user to lose money.

Tools Used
Foundry, VsCode

Recommended mitigation

Cast the msg.sender global variable to payable.

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,) = payable(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");
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;
require(totalPaid >= requiredCost, "Insufficient ETH sent to complete purchase");
// Transfer the NFT to the buyer
_transfer(address(this), msg.sender, tokenId);
// Clean up storage
delete pendingNFTs[tokenId];
delete pendingNFTsAmountPaid[tokenId];
delete tokenIdToTreatName[tokenId];
// Refund excess ETH if any
if (totalPaid > requiredCost) {
uint256 refund = totalPaid - requiredCost;
+ (bool refundSuccess,) = payable(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

Support

FAQs

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