Trick or Treat
First Flight #27
Beginner FriendlyFoundry
100 EXP
View results
Previous Next
Submission Details
Severity: low
Invalid

Logic error

predator

Summary

Logic error occurs in SpookySwap::setTreatCost function, that will not allow owner to change SpookySwap::Treat::cost, if it was assigned as 0 (by mistake)

Vulnerability Details

The SpookySwap::setTreatCost function will not allow to change SpookySwap::Treat::cost, if it was assigned as 0 (by mistake), and as a result, this treat will not be usable for Trick or Treat.

function setTreatCost(string memory _treatName, uint256 _cost) public onlyOwner {
@> require(treatList[_treatName].cost > 0, "Treat must cost something.");
treatList[_treatName].cost = _cost;
}

Proof of Concepts:

  1. Owner deploys contract with pre-revealed array of treats.

  2. By mistake, in one of the Treat::cost revealed 0.

  3. Calling SpookySwap::setTreatCost function, will only revert, not giving an opportunity to change cost.

It is possible to check it, launch the following code from test folder.

Code
contract TrickOrTreatTest is Test {
SpookySwap.Treat[] public treats;
SpookySwap public trickOrTreat;
​
address public owner = makeAddr("owner");
address public user = makeAddr("user");
​
function setUp() public {
treats.push(SpookySwap.Treat("treat1", 1 ether, "uri1"));
treats.push(SpookySwap.Treat("treat2", 2 ether, "uri2"));
treats.push(SpookySwap.Treat("treat3", 0 ether, "uri3"));
trickOrTreat = new SpookySwap(treats);
}
​
function test_ZeroCostTreat() public {
vm.prank(owner);
vm.expectRevert();
trickOrTreat.setTreatCost("treat3", 3 ether);
}
}
​

Impact

Human factor, assigning Trait::cost as 0, may impact on diversity of treats that players will like to mint.

Tools Used

Foundry

Recommendations

There are a few ways how to mitigate this issue:

To prevent this mistake we can make small modification in SpookySwap::setTreatCost:

function setTreatCost(string memory _treatName, uint256 _cost) public onlyOwner {
- require(treatList[_treatName].cost > 0, "Treat must cost something.");
treatList[_treatName].cost = _cost;
+ require(treatList[_treatName].cost > 0, "Treat must cost something.");
}

This will allow to change cost even if owner, by mistake, assinged it as 0: in deployment process or calling SpookySwap::setTreatCost to change cost price.

Another way to make modification of SpookySwap::addTreat:

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

The contract deployment will fail, and will not be created on the blockchain.

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.