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 {
@> treatList[_name] = Treat(_name, _rate, _metadataURI);
treatNames.push(_name);
emit TreatAdded(_name, _rate, _metadataURI);
}
function setTreatCost(string memory _treatName, uint256 _cost) public onlyOwner {
@> require(treatList[_treatName].cost > 0, "Treat must cost something.");
@> treatList[_treatName].cost = _cost;
}
function trickOrTreat(string memory _treatName) public payable nonReentrant {
Treat memory treat = treatList[_treatName];
@> 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
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;
}