Trick or Treat

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

Unable to Set Treat Cost for Treats Initialized with Zero Cost Value, Rendering the Treat NFT Unclaimable

Summary

The setTreatCost function's zero-cost validation check prevents updating costs for treats initialized at zero value, making these NFTs permanently unclaimable and creating "dead" tokens in the collection.

Vulnerability Details

The TrickOrTreat::setTreatCost function incorrectly requires the existing treat cost to be greater than zero before allowing cost updates, which permanently locks any treat initialized with a zero cost from ever being updated, making these NFTs permanently unclaimable. This vulnerability effectively creates "dead" NFTs in the collection when treats are initially set with zero cost, as the owner cannot update their prices, thus breaking core contract functionality.

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

Impact

Any NFT treat initialized with zero cost becomes permanently unclaimable and unusable, resulting in a permanent loss of functionality for affected tokens and potential revenue loss for the platform.

Tools Used

Foundry Test, Manual Review

Proof of Concept:

To create a test folder and a file named TestTrickOrTreat.t.sol for your Foundry project, you can follow these steps:

  • Create the test folder: Navigate to your project directory and create a test folder.

  • Create the TestTrickOrTreat.t.sol file: Inside the test folder, create a file named TestTrickOrTreat.t.sol.

  • Add the Foundry test code: Here’s an example of what you might include in your TestTrickOrTreat.t.sol file:

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.24;
import {Test, console} from "forge-std/Test.sol";
import {SpookySwap} from "../src/TrickOrTreat.sol";
contract TrickOrTreatTest is Test {
SpookySwap public spookySwap;
address owner;
function setUp() public {
// Create an array of treats for initialization
owner = makeAddr("owner");
SpookySwap.Treat[] memory treats = new SpookySwap.Treat[]();
// Initialize treats with test data
treats[0] = SpookySwap.Treat({
name: "Ghostly Gummies",
cost: 0.01 ether,
metadataURI: "ipfs://QmX1x2x3/1"
});
treats[1] = SpookySwap.Treat({
name: "Witch's Brew",
cost: 0.02 ether,
metadataURI: "ipfs://QmX1x2x3/2"
});
treats[2] = SpookySwap.Treat({
name: "Zombie Zephyr",
cost: 0.015 ether,
metadataURI: "ipfs://QmX1x2x3/3"
});
// Deploy the contract with initial treats
vm.prank(owner);
spookySwap = new SpookySwap(treats);
}
function test_RevertsetTreatCost_WhenInitalTreatCostEqualsZero() public {
string memory name = "Holy Valley";
uint256 cost = 0; //initilized treat with zero cost
string memory metadataURI = "ipfs://QmX1x2x3/4";
uint256 newTreatCost = 0.12 ether; // New Treat Cost
vm.startPrank(owner);
spookySwap.addTreat(name, cost, metadataURI); // adding a new Treat with zero cost
string memory treatName = spookySwap.getTreats()[3];
vm.expectRevert();
spookySwap.setTreatCost(treatName, newTreatCost);
vm.stopPrank();
}
}
  • Then run the test on your terminal:

forge test --mt test_RevertsetTreatCost_WhenInitalTreatCostEqualsZero

Recommendations

To mitigate the issue of ensuring that the treat cost is not set to zero, changing the comparison from > to >= is a sensible approach. This allows the cost to be zero if the intention is to allow free treats or reset the cost.

Here’s how you can modify the function:

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;
}
Updates

Appeal created

bube Lead Judge about 1 year 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.