Trick or Treat

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

In `SpookySwap::constructor` and `SpookySwap::addTreat`, if cost default setting is zero, then function `setTreatCost` & `trickOrTreat` will not be able to run.

Summary

Inside both functions: setTreatCost & trickOrTreat.
There are check of treat's cost should greater than 0.
If the owner accidently setting a 0 cost when deploy contract or adding treat by addTreat function.
It will led some function can't run.

Vulnerability Details

Here is a test of adding a treat with 0 cost.
Then setTreatCost will revert !

POC

function test_addTreatWithZeroCost() public {
string memory newName = "newTreat";
uint256 newCost = 0;
string memory newUri = "new_uri_200";
vm.prank(deployer);
swap.addTreat(newName, newCost, newUri);
(, uint256 expectedCost, ) = swap.treatList(newName);
assert(expectedCost == 0);
vm.prank(deployer);
vm.expectRevert("Treat must cost something.");
swap.setTreatCost(newName, 100);
// when accidentally add treat with 0 cost,
// it can not setTreatCost() later! nor do trickOrTreat()
// because of require(cost > 0)
}

Impact

We can see that inside setTreatCost & trickOrTreat.
require(treat.cost > 0)
If one treat's cost is 0, then these two function will useless.
Because it can not change cost, also can not do trickOrTreat function at all.
It means this cost 0 treat, will never be claimed by anyone.

The likelihood might be low/medium, it only happen when owner accidently add treat's cost wrong since only onwer can add a treat.
Impact should be high, because if it really happen, that treat will never be claimed.

Tools Used

Manually

Recommendations

add check before create any treat. In constructor and in addTreat as following:

constructor(Treat[] memory treats) ERC721("SpookyTreats", "SPKY") {
nextTokenId = 1;
for (uint256 i = 0; i < treats.length; i++) {
+ if(treat[i].cost == 0) revert SpookySwap__CostMustGreaterThanZero();
addTreat(treats[i].name, treats[i].cost, treats[i].metadataURI);
}
}
function addTreat(
string memory _name,
uint256 _rate,
string memory _metadataURI
) public onlyOwner {
+ if(_rate == 0) revert SpookySwap__CostMustGreaterThanZero();
treatList[_name] = Treat(_name, _rate, _metadataURI);
treatNames.push(_name);
emit TreatAdded(_name, _rate, _metadataURI);
}
Updates

Appeal created

bube Lead Judge 8 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.