Trick or Treat

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

No check for existing treats in function `SpookySwap::addTreat()`

Summary

There is no check if the treat added by SpookySwap::addTreat() already exists. Hence it is possible to overwrite existing treats with new ones. On the other hand the `treatNames' variable will still keep track of the old (non-existent) treat

Vulnerability Details

Before adding a new treat to a contract, you should check to see if a treat with the same name already exists and decide whether to overwrite or change the name of the treat.

Impact

  1. Users may see more treat than there actually are

  2. Owner may accidentally overwrite uri and cost of existing treat

Tools Used

Foundry test as below

// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.13;
import {Test, console} from "forge-std/Test.sol";
import {SpookySwap} from "src/TrickOrTreat.sol";
contract SpookySwapTest is Test {
SpookySwap public spookySwap;
SpookySwap.Treat[] public treatList;
function setUp() public {
_addTreats();
spookySwap = new SpookySwap(treatList);
}
function test_two_treat_same_name() public view {
string[] memory treats = spookySwap.getTreats();
//Two treats are supposed to be stored
assertEq(treats[0], "Zombie Brain Cupcakes");
assertEq(treats[4], "Zombie Brain Cupcakes");
//But only one (the last one) really is.
(string memory name, uint256 cost, string memory uri) = spookySwap.treatList("Zombie Brain Cupcakes");
assertEq(name, "Zombie Brain Cupcakes");
assertEq(cost, 0.002 ether);
assertEq(uri, "The same string as first treat");
}
///////////////////////////////////////////////// HELPER FUNCTIONS /////////////////////////////////////////////////
function _addTreats() private {
treatList.push(SpookySwap.Treat("Zombie Brain Cupcakes", 0.001 ether, "Yammy!"));
treatList.push(SpookySwap.Treat("Vampire Poke Cake", 0.005 ether, "Is it blood?"));
treatList.push(SpookySwap.Treat("Witch Finger Cookies", 0.0002 ether, "Oh no!"));
treatList.push(SpookySwap.Treat("Pumpkin Patch Brownies", 0.001 ether, "Chocolade!"));
treatList.push(SpookySwap.Treat("Zombie Brain Cupcakes", 0.002 ether, "The same string as first treat")); //almost the same as first!
}
}

Recommendations

Add code to the SpookySwap::addTreat() function that checks if treats with the same name exist, according to logic set by the owner.

Updates

Appeal created

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