Trick or Treat

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

Incorrect / Insufficient checks in `setTreatCost`

Summary

  • Root Cause: setTreatCost has incorrect validation logic

  • Impact: Treats could be set to cost 0, contrary to the requirement check.

Vulnerability Details

https://github.com/Cyfrin/2024-10-trick-or-treat/blob/main/src/TrickOrTreat.sol#L43-L46

The setTreatCost function has a logical error in its validation:

function setTreatCost(string memory _treatName, uint256 _cost) public onlyOwner {
require(treatList[_treatName].cost > 0, "Treat must cost something.");
treatList[_treatName].cost = _cost; // Can set to 0 after check!
}

The function checks if the current cost is > 0, but doesn't validate the new cost, allowing setting a zero cost.

POC

function testSetTreatCostToZero() public {
spookySwap.addTreat("Candy", 1 ether, "uri");
spookySwap.setTreatCost("Candy", 0); // This works!
// Now treats can be minted for free
assertEq(spookySwap.treatList("Candy").cost, 0);
}

Impact

cost can be set to zero, allows users to get free nft

Tools Used

Manual Review

Recommendations

function setTreatCost(string memory _treatName, uint256 _cost) public onlyOwner {
require(_cost > 0, "New cost must be greater than 0");
require(treatList[_treatName].cost > 0, "Treat does not exist");
treatList[_treatName].cost = _cost;
}
Updates

Appeal created

bube Lead Judge 9 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

[invalid] Zero treat cost

The cost of the treat is set only by the owner (in the constructor, in addTreat and in setTreatCost). That means the cost of the treat will always be greater than zero.

Support

FAQs

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