Summary
The setTreatCost
function in SpookySwap contract contains a logical error in its validation check where it validates the old cost value instead of the new one, allowing setting zero or negative costs for treats.
Vulnerability Details
The current implementation:
function setTreatCost(string memory _treatName, uint256 _cost) public onlyOwner {
require(treatList[_treatName].cost > 0, "Treat must cost something.");
treatList[_treatName].cost = _cost;
}
The issue lies in the validation logic:
require(treatList[_treatName].cost > 0)
checks the CURRENT cost
After passing this check, ANY new cost value is accepted
No validation is performed on the _cost
parameter
Example exploit:
spookySwap.addTreat("Candy", 1 ether, "uri");
spookySwap.setTreatCost("Candy", 0);
spookySwap.setTreatCost("Candy", type(uint256).max);
Proof of Concept:
contract SpookySwapTest {
function testIncorrectCostValidation() public {
Treat[] memory treats = new Treat[]();
treats[0] = Treat("Candy", 1 ether, "uri");
SpookySwap spooky = new SpookySwap(treats);
spooky.setTreatCost("Candy", 0);
assertEq(spooky.treatList("Candy").cost, 0);
}
}
Impact
HIGH severity
Allows setting zero costs for treats
Can break the economic model of the platform
Possible front-running opportunities
Potential loss of revenue
Real-world attack scenarios:
Owner attempts to update price from 1 ETH to 2 ETH
Attacker front-runs and sets price to 0
Multiple users can mint for free before owner notices
Economic damage to the platform
Tools Used
Recommendations
Correct the validation logic to check the new cost:
function setTreatCost(string memory _treatName, uint256 _cost) public onlyOwner {
require(treatList[_treatName].cost > 0, "Treat does not exist");
require(_cost > 0, "New cost must be greater than 0");
treatList[_treatName].cost = _cost;
emit TreatCostUpdated(_treatName, _cost);
}
Add additional safeguards:
function setTreatCost(string memory _treatName, uint256 _cost) public onlyOwner {
Treat storage treat = treatList[_treatName];
require(treat.cost > 0, "Treat does not exist");
require(_cost > 0, "New cost must be greater than 0");
uint256 maxChange = treat.cost * 2;
uint256 minChange = treat.cost / 2;
require(_cost <= maxChange, "Price increase too high");
require(_cost >= minChange, "Price decrease too high");
treat.cost = _cost;
emit TreatCostUpdated(_treatName, _cost);
}
Implement a time-delay for price changes:
struct TreatPrice {
uint256 currentCost;
uint256 pendingCost;
uint256 updateTimestamp;
}
mapping(string => TreatPrice) public treatPrices;
uint256 public constant PRICE_UPDATE_DELAY = 1 days;
function scheduleTreatCostUpdate(string memory _treatName, uint256 _newCost) public onlyOwner {
require(_newCost > 0, "New cost must be greater than 0");
require(treatList[_treatName].cost > 0, "Treat does not exist");
treatPrices[_treatName] = TreatPrice({
currentCost: treatList[_treatName].cost,
pendingCost: _newCost,
updateTimestamp: block.timestamp + PRICE_UPDATE_DELAY
});
emit TreatCostUpdateScheduled(_treatName, _newCost, block.timestamp + PRICE_UPDATE_DELAY);
}
function executeTreatCostUpdate(string memory _treatName) public {
TreatPrice storage priceInfo = treatPrices[_treatName];
require(block.timestamp >= priceInfo.updateTimestamp, "Update not ready");
require(priceInfo.pendingCost > 0, "No update scheduled");
treatList[_treatName].cost = priceInfo.pendingCost;
delete treatPrices[_treatName];
emit TreatCostUpdated(_treatName, priceInfo.pendingCost);
}
The proposed changes provide:
Correct validation of new cost values
Protection against extreme price changes
Time-delay for price updates
Clear event emission for tracking
Ability to cancel pending price changes
Additional considerations:
Add price change limits based on time periods
Implement emergency price change mechanism
Add multi-signature requirement for price changes
Consider using price oracles for automated adjustments
This fix transforms what could have been a critical economic vulnerability into a secure price management system.