The trickOrTreat function in the SpookySwap contract handles NFT purchases but lacks a mechanism to release funds and NFTs when a purchase remains incomplete. When a user initiates a purchase at double price (trick scenario) without completing it and never calls resolveTrick(), both funds and NFTs can remain indefinitely locked in the contract. This issue can lead to blocked funds and potential loss of user funds.
The vulnerability arises from incomplete handling of NFT purchases, particularly when the trickOrTreat function sets an NFT purchase to a pending state. This can occur if the user does not complete the second half of the purchase by sending the additional required ETH in the resolveTrick function. Without an expiry mechanism for pending purchases, these NFTs remain indefinitely locked, along with the funds the user initially paid. This will raise following key issues
Introduce new function refundExpiredPending in which after a set expired date the owner will then release the refund to the user with following aspects:
pragma solidity ^0.8.24;
import "lib/openzeppelin-contracts/contracts/token/ERC721/extensions/ERC721URIStorage.sol";
import "lib/openzeppelin-contracts/contracts/access/Ownable.sol";
import "lib/openzeppelin-contracts/contracts/utils/ReentrancyGuard.sol";
contract SpookySwap is ERC721URIStorage, Ownable(msg.sender), ReentrancyGuard {
uint256 public nextTokenId;
uint256 public constant PENDING_EXPIRY = 1 days;
mapping(string => Treat) public treatList;
string[] public treatNames;
struct Treat {
string name;
uint256 cost;
string metadataURI;
}
mapping(uint256 => address) public pendingNFTs;
mapping(uint256 => uint256) public pendingNFTsAmountPaid;
mapping(uint256 => uint256) public pendingNFTsTimestamp;
mapping(uint256 => string) public tokenIdToTreatName;
event TreatAdded(string name, uint256 cost, string metadataURI);
event Swapped(address indexed user, string treatName, uint256 tokenId);
event FeeWithdrawn(address owner, uint256 amount);
+event PendingExpired(uint256 tokenId, address buyer, uint256 amountPaid);
constructor(Treat[] memory treats) ERC721("SpookyTreats", "SPKY") {
nextTokenId = 1;
for (uint256 i = 0; i < treats.length; i++) {
addTreat(treats[i].name, treats[i].cost, treats[i].metadataURI);
}
}
function addTreat(string memory _name, uint256 _rate, string memory _metadataURI) public onlyOwner {
treatList[_name] = Treat(_name, _rate, _metadataURI);
treatNames.push(_name);
emit TreatAdded(_name, _rate, _metadataURI);
}
function setTreatCost(string memory _treatName, uint256 _cost) public onlyOwner {
require(treatList[_treatName].cost > 0, "Treat must cost something.");
treatList[_treatName].cost = _cost;
}
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;
uint256 random =
uint256(keccak256(abi.encodePacked(block.timestamp, msg.sender, nextTokenId, block.prevrandao))) % 1000 + 1;
if (random == 1) {
costMultiplierNumerator = 1;
costMultiplierDenominator = 2;
} else if (random == 2) {
costMultiplierNumerator = 2;
costMultiplierDenominator = 1;
}
uint256 requiredCost = (treat.cost * costMultiplierNumerator) / costMultiplierDenominator;
if (costMultiplierNumerator == 2 && costMultiplierDenominator == 1) {
if (msg.value >= requiredCost) {
mintTreat(msg.sender, treat);
} else {
uint256 tokenId = nextTokenId;
_mint(address(this), tokenId);
_setTokenURI(tokenId, treat.metadataURI);
nextTokenId += 1;
pendingNFTs[tokenId] = msg.sender;
pendingNFTsAmountPaid[tokenId] = msg.value;
pendingNFTsTimestamp[tokenId] = block.timestamp;
tokenIdToTreatName[tokenId] = _treatName;
emit Swapped(msg.sender, _treatName, tokenId);
}
} else {
require(msg.value >= requiredCost, "Insufficient ETH sent for treat");
mintTreat(msg.sender, treat);
}
if (msg.value > requiredCost) {
uint256 refund = msg.value - requiredCost;
(bool refundSuccess,) = msg.sender.call{value: refund}("");
require(refundSuccess, "Refund failed");
}
}
function mintTreat(address recipient, Treat memory treat) internal {
uint256 tokenId = nextTokenId;
_mint(recipient, tokenId);
_setTokenURI(tokenId, treat.metadataURI);
nextTokenId += 1;
emit Swapped(recipient, treat.name, tokenId);
}
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;
uint256 amountPaid = pendingNFTsAmountPaid[tokenId];
uint256 totalPaid = amountPaid + msg.value;
require(totalPaid >= requiredCost, "Insufficient ETH sent to complete purchase");
_transfer(address(this), msg.sender, tokenId);
delete pendingNFTs[tokenId];
delete pendingNFTsAmountPaid[tokenId];
delete pendingNFTsTimestamp[tokenId];
delete tokenIdToTreatName[tokenId];
if (totalPaid > requiredCost) {
uint256 refund = totalPaid - requiredCost;
(bool refundSuccess,) = msg.sender.call{value: refund}("");
require(refundSuccess, "Refund failed");
}
}
+function refundExpiredPending(uint256 tokenId) public onlyOwner nonReentrant {
require(pendingNFTs[tokenId] != address(0), "No pending NFT for tokenId");
require(block.timestamp > pendingNFTsTimestamp[tokenId] + PENDING_EXPIRY, "Pending purchase not expired");
address buyer = pendingNFTs[tokenId];
uint256 amountPaid = pendingNFTsAmountPaid[tokenId];
delete pendingNFTs[tokenId];
delete pendingNFTsAmountPaid[tokenId];
delete tokenIdToTreatName[tokenId];
delete pendingNFTsTimestamp[tokenId];
(bool success, ) = buyer.call{value: amountPaid}("");
require(success, "Refund failed");
emit PendingExpired(tokenId, buyer, amountPaid);
}
function withdrawFees() public onlyOwner {
uint256 balance = address(this).balance;
payable(owner()).transfer(balance);
emit FeeWithdrawn(owner(), balance);
}
function getTreats() public view returns (string[] memory) {
return treatNames;
}
function changeOwner(address _newOwner) public onlyOwner {
transferOwnership(_newOwner);
}
}
