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

The function `SpookySwap::setTreatCost` does not validate the parameter `_cost`, allowing the cost of Treats to be set to zero and stay like that forever.

xilobytes

Description

Inside the SpookySwap::setTreatCost function the parameter _cost is never checked to be greater than zero, and since the require statement, require(treatList[_treatName].cost > 0, "Treat must cost something.");, checks that the Treat must have had a value greater than zero alredy, it prevents the Treat cost to be updated again.

Impact

The function allows that the cost of any Treat can be updated to zero, hence minting these kinds of NFTs will be for free. And since the require(treatList[_treatName].cost > 0, "Treat must cost something."); validates that in order to update the cost of a Treat, its previous value must have been greater than zero before, prevents that a new cost could be set; making the Treat free to mint forever.

Proof of Concepts

Create a test file named TrickOrTreat.t.sol and include the following code.

//SPDX-License-Identifier: MIT
​
pragma solidity ^0.8.24;
​
import {Test} from "forge-std/Test.sol";
import {SpookySwap} from "../src/TrickOrTreat.sol";
​
contract SpookySwapTest is Test{
SpookySwap spooky;
uint256 public initialTreatCost = 2e18;
uint256 public userBalance = 5e18;
string public metadataURI = "SpookySwapNFT";
address public user1 = makeAddr("user1");
​
function setUp() public{
//Create the Treats and push them into the array
SpookySwap.Treat[] memory treats = new SpookySwap.Treat[]();
treats[0] = SpookySwap.Treat("treat1", initialTreatCost, metadataURI);
treats[1] = SpookySwap.Treat("treat2", initialTreatCost, metadataURI);
treats[2] = SpookySwap.Treat("treat3", initialTreatCost, metadataURI);
// Create and initialize SpookySwap
spooky = new SpookySwap(treats);
// Fund user accounts
vm.deal(user1, userBalance);
vm.deal(user2, userBalance);
}
​
function testCannotUpdateCostIfItWasPreviouslySetToZero() public{
// Update the cost of treat1 from 2e18 to zero
spooky.setTreatCost("treat1", 0);
​
//Updating the cost of treat1 once again is not longer possible, making it free to mint forever
vm.expectRevert("Treat must cost something.");
spooky.setTreatCost("treat1", initialTreatCost);
}
}

Tools Used
Foundry, VsCode

Recommended mitigation

Add also a validation in the function to check if the new cost is greater than zero.

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.