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

addTreat function can create duplicate Treats

delinked

Summary

The addTreat function does not check for duplicate names, allowing treats with the same name, cost, and metadata URI to be added multiple times. This leads to multiple identical entries in treatNames and redundant storage in treatList.

Proof of Code

The following test attempts to see if when adding a new treat twice (The same addTreat line was added in the setUp function), we would expect that the length of the treatNames array would still be 1, since we are sending the same value, but due to not having a duplicate check, the length returned is 2.

function testOverwriteTreatWithDuplicateName() public {
// Adding the same treat with identical name, cost, and metadata URI
vm.prank(owner);
spookySwap.addTreat("CandyCorn", 1 ether, "ipfs:://Qm...CandyCorn");
​
// Check length of treatNames array and expect only one instance of "CandyCorn"
uint256 treatNamesLength = spookySwap.getTreats().length;
​
// Log for proof of code
console.log("Expected treatNames length: 1, Actual length:", treatNamesLength);
​
// Assert the treatNames array should only have one "CandyCorn" entry
assertEq(treatNamesLength, 1, "There should be only one instance of 'CandyCorn' in treatNames.");
}

Below is the output of the console.log:

Logs:
Expected treatNames length: 1, Actual length: 2

Impact

  • This vulnerability allows for duplicate treat entries, which could lead to a bloated treatNames array and potential confusion around unique treat retrieval.

  • Duplicate entries also increase storage usage and gas costs in transactions related to treat retrieval and minting, particularly for repeated retrieval or iteration over treats.

  • Although the function is restricted to onlyOwner, duplicate entries due to error or oversight could degrade contract performance and user experience.

Tools Used

Manual review and unit tests with Foundry.

Recommendations

Add a uniqueness check in addTreat to verify that no treat with the specified name already exists in treatList before adding a new treat. This could be implemented with a simple require statement:

function addTreat(string memory _name, uint256 _rate, string memory _metadataURI) public onlyOwner {
@> require(treatList[_name].cost == 0, "Treat with this name already exists.");
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] 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.