Trick or Treat

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

Cost of a Treat with Zero Initial Value Cannot Be Updated and is Unclaimable by Users

Description: When adding a new treat using SpookySwap::addTreat, there is no verification to ensure the cost is non-zero. If a treat is added with a cost of 0, attempts to later update the cost via the SpookySwap::setTreatCost function will fail due to the check require(treatList[_treatName].cost > 0, "Treat must cost something.");. Since the treat was initialized with zero, this check will trigger a revert.
Additionally, when a user tries to claim this treat by calling SpookySwap::trickOrTreat, it will also fail because of the require(treat.cost > 0, "Treat cost not set."); check.

Impact: The owner will be unable to update the treat's cost in the future, rendering the treat useless while it continues to occupy storage. From the users' perspective, they will also be unable to claim this treat.

Proof of Concept:

  1. The owner creates a new treat with a cost of 0 ether.

  2. The owner calls SpookySwap::setTreatCost to update the cost to a value greater than 0.

  3. The SpookySwap::setTreatCost function reverts with the error message: "Treat must cost something."

Proof of Code:

Create a directory for the test file at the root level of the repository as test/unit. Create a new file named TrickOrTreatTest.t.sol inside this directory and paste the following code into this file.

// SPDX-License-Identifier: MIT
pragma solidity 0.8.24;
import {Test, console2} from "forge-std/Test.sol";
import {SpookySwap} from "../../src/TrickOrTreat.sol";
contract TrickOrTreatTest is Test {
SpookySwap spookySwap;
SpookySwap.Treat treat;
SpookySwap.Treat[] treatsList;
address owner = makeAddr("owner");
address nonowner = makeAddr("nonowner");
function setUp() public {
treat.name = "GoinLiveTreat";
treat.cost = 1 ether;
treat.metadataURI = "test.metadata.com";
treatsList.push(treat);
vm.prank(owner);
spookySwap = new SpookySwap(treatsList);
console2.log("SpookySwap address: ", address(spookySwap));
}
function testATreatWithZeroCostCanNotBeUpdatedOrClaimed() public {
string memory treatName = "TreatWZC";
string memory treatURI = "TreatWZC.com";
vm.startPrank(owner);
// Owner adds a Treat with zero cost
spookySwap.addTreat(treatName, 0 ether, treatURI);
vm.expectRevert("Treat must cost something.");
// Owner tries to update the cost to a value > 0
spookySwap.setTreatCost(treatName, 1);
vm.stopPrank();
// User is also unable to claim the treat
vm.prank(nonowner);
vm.expectRevert("Treat cost not set.");
spookySwap.trickOrTreat(treatName);
}
}

Recommended Mitigation: Two potential solutions exist to mitigate this issue.

First Solution:

Update the SpookySwap::setTreatCost function to check for a cost of 0 as well.

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

Second Solution:

Update the SpookySwap::addTreat function to prevent the creation of a new treat with a zero value.

function addTreat(string memory _name, uint256 _rate, string memory _metadataURI) public onlyOwner {
treatList[_name] = Treat(_name, _rate, _metadataURI);
+ if(_rate == 0){
+ revert("Adding a treat with zero cost value is not permitted");
+ }
treatNames.push(_name);
emit TreatAdded(_name, _rate, _metadataURI);
}
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.