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

Missing parameters checkers in function `SpookySwap::addTreat` allowing the addition of treats with duplicated names, and also treats with zero cost. This may cause the overwriting of already created treats, and setting treats for free.

xilobytes

Description

The function SpookySwap::addTreat doesn't validate if a treat has been already created with the _name parameter, also it doesn't check if the _rate parameter is greater than zero.

Impact

An already created treat can be overwritten by a totally new treat with the same name, but different cost and metadata. This would make the first treat no longer valid for future possible mintings. Also, allowing a treat to have a cost of zero would make the protocol to lose money as the minting would be for free.

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 testAddTreatsWithAlreadyUsedNames() public{
string memory newTreat = "treat1";
uint256 newTreatCost = 3e18;
string memory newMetadataURI = "SpookySwapNFTTreat4";
​
spooky.addTreat(newTreat, newTreatCost, newMetadataURI);
//Getting the values of treat1
(string memory name, uint256 cost, string memory uri) = spooky.treatList("treat1");
​
//Confirming the treat values of treat1 have been overwritten by treat4
assertEq(name, newTreat);
assertEq(cost, newTreatCost);
assertEq(uri, newMetadataURI);
}
}

Tools Used
Foundry, VsCode

Recommended mitigation

Add _name and _rate parameter checkers in the SpookySwap::addTreat function to verify no Treat has been created with that name already and that the cost is not set to zero.

function addTreat(string memory _name, uint256 _rate, string memory _metadataURI) public onlyOwner {
+ require(treatList[_name].cost == 0, "Treat already exists");
+ require(_rate > 0, "Cost must be greater than zero");
treatList[_name] = Treat(_name, _rate, _metadataURI);
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.

[invalid] Duplicate treats

The function `addTreat` is called by the owner. The owner is trusted. There will be no duplicates.

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.