Trick or Treat

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

Function `setTreatCost` allow setting the cost to 0 ether which makes the treat unusable

Summary

The owner can call function setTreatCost(string, uint256) by passing 0 ether to the _costparameter which will break the treat and the cost cannot be changed anymore.

Vulnerability Details

This means the owner can call setTreatCost(string, uint256)function and set the cost to 0 ether.

After that if the owner tries to change the cost for this treat again it will fails because of the following line of code in the function setTreatCost(string, uint256):

require(treatList[_treatName].cost > 0, "Treat must cost something.");

This line checks the current cost of the treat to be > 0.

Then users will never be able to mint NFT for this treat, because of the function trickOrTreat(string)checks if the cost of the treat is > 0.

Another case will be if there are users should pay 2x price of the treat, but did not call the resolveTrickfunction yet.

If the cost is changed to 0 between the minting of the NFT and the call to the resolveTrickfunction, these users will be able to receive the NFT for a lot cheaper than the other users. (The users which claimed the NFT and paid full or 2x price before changing the cost to 0 ether)

This is because in the resolveTrickfunction at the bottom is this code which will refund ETH to the players, so basically the players will pay only gas fee for this NFT.

// Refund excess ETH if any
if (totalPaid > requiredCost) {
uint256 refund = totalPaid - requiredCost;
(bool refundSuccess, ) = msg.sender.call{value: refund}("");
require(refundSuccess, "Refund failed");
}

POC

Specific example:

  1. User BoB tries his chance with the trickOrTreat function to mint NFT. He is unlucky and he has to pay 2x the price, however paid only the gas price since there is no validation about the value of the transaction in case of 2x.
    Because of that he paid only gas price and the NFT was minted to the contract.

  2. The owner changes the cost of the treat to 0 by calling setTreatCost function

  3. Bob decides that he wants to get the NFT because he really likes it, so he calls function resolveTrick

  4. Bob receives the NFT for free (only paying gas price twice).

Add the following test to the class TestTrickOrTreat.t.sol (test class is not part of the project so you have to create it):

function setUp() public {
trickOrTreat = new SpookySwap(new SpookySwap.Treat[](0));
}
function test_cannotChangePriceOfTreat2() external {
// Create treat with cost of 1 ether
trickOrTreat.addTreat("testTreat", 1 ether, "testUrl");
// For some reason the owner decides to update the cost to 0 ether
trickOrTreat.setTreatCost("testTreat", 0 ether);
// Not able to change the cost after that
vm.expectRevert();
trickOrTreat.setTreatCost("testTreat", 1 ether);
}

Impact

This makes the treat unusable and also allow certain users(players) to mint the NFT for a lot cheaper than others.

Tools Used

foundry

Recommendations

In the function setTreatCostcheck if the parameter _cost > 0instead of checking the current cost function:

function setTreatCost(
string memory _treatName,
uint256 _cost
) public onlyOwner {
- require(treatList[_treatName].cost > 0, "Treat must cost something.");
+ require(_cost > 0, "Treat must cost something.");
// @audit - validate the _cost parameter as well, otherwise it would be impossible to trickOrTreat that treat
// because of the check in the function trickOrTreat => require(treat.cost > 0, "Treat cost not set.");
treatList[_treatName].cost = _cost;
}
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.