Trick or Treat

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

Lack of "zero-cost" check by `SpookySwap:addTreat`, disables `Treats` of being `SpookySwap:setTreatCost` and `SpookySwap:trickOrTreat`.

Summary

By adding a new Treat either by contract construction or calling SpookySwap:addTreat no checks for zero-prices are set. Consequently by calling SpookySwap:setTreatCost or SpookySwap:trickOrTreat, only those with prices greater than 0 will proceed. Moreover, updates of Treats to zero-cost aren't checked too.

Vulnerability Details

function addTreat(string memory _name, uint256 _rate, string memory _metadataURI) public onlyOwner {
// _rate = 0 is set
@> treatList[_name] = Treat(_name, _rate, _metadataURI);
treatNames.push(_name);
emit TreatAdded(_name, _rate, _metadataURI);
}
function setTreatCost(string memory _treatName, uint256 _cost) public onlyOwner {
// treatList[_treatName].cost == 0; stuck or disabled
@> require(treatList[_treatName].cost > 0, "Treat must cost something.");
// _rate = 0 is set
@> treatList[_treatName].cost = _cost;
}
function trickOrTreat(string memory _treatName) public payable nonReentrant {
Treat memory treat = treatList[_treatName];
//treat.cost == 0; disabled
@> require(treat.cost > 0, "Treat cost not set.");
/*
...impl...
*/
}

Impact

stuck Treats in the Protocol, disabled of being updated nor trickedOrTreated, having them unusable which effectively waste protocol's resources. although it does not risk Protocol:funds.

Tools Used

  • Manual Review

  • Foundry testing tool

function testLackOfZeroCostVerification() public {
vm.expectEmit();
emit TreatAdded("candy", 0, "");
protocol.addTreat("candy", 0, "");
vm.expectRevert(bytes("Treat must cost something."));
protocol.setTreatCost("candy", 1 ether);
vm.expectRevert(bytes("Treat cost not set."));
protocol.trickOrTreat{ value: 0.1 ether }("candy");
}

Recommendations

  • Add zero-check validation by SpookySwap:addTreat and SpookySwap:setTreatCost.

function addTreat(string memory _name, uint256 _rate, string memory _metadataURI) public onlyOwner {
+ require(_rate > 0);
treatList[_name] = Treat(_name, _rate, _metadataURI);
treatNames.push(_name);
emit TreatAdded(_name, _rate, _metadataURI);
}
function setTreatCost(string memory _treatName, uint256 _cost) public onlyOwner {
+ require(_cost > 0);
require(treatList[_treatName].cost > 0, "Treat must cost something.");
treatList[_treatName].cost = _cost;
}
Updates

Appeal created

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