Trick or Treat

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

Treat Name Collisions Create Data Structure Inconsistency

Summary

The addTreat() function allows duplicate treat names, creating an inconsistency between the treats array and treatList mapping.

Vulnerability Details

Location: src/TrickOrTreat.sol:addTreat()

Proof of Concept:

function testTreatOverwrite() public {
// Start with owner permissions
vm.startPrank(owner);
// Add initial treat
spookySwap.addTreat("SpecialCandy", 1 ether, "uri1");
// Verify initial treat details
(, uint256 cost1, string memory uri1) = spookySwap.treatList("SpecialCandy");
assertEq(cost1, 1 ether, "Initial cost should be 1 ether");
assertEq(uri1, "uri1", "Initial URI should match");
// Add the same treat with different parameters
spookySwap.addTreat("SpecialCandy", 2 ether, "uri2");
// Verify the treat mapping was updated
(, uint256 cost2, string memory uri2) = spookySwap.treatList("SpecialCandy");
assertEq(cost2, 2 ether, "Cost should be updated to 2 ether");
assertEq(uri2, "uri2", "URI should be updated to uri2");
// Get all treats and demonstrate the duplicate entries
string[] memory allTreats = spookySwap.getTreats();
// Count occurrences of "SpecialCandy"
uint256 specialCandyCount = 0;
for(uint256 i = 0; i < allTreats.length; i++) {
if (keccak256(bytes(allTreats[i])) == keccak256(bytes("SpecialCandy"))) {
specialCandyCount++;
console.log("Found SpecialCandy at index:", i);
}
}
// This assertion will fail, showing the vulnerability
assertEq(specialCandyCount, 2, "SpecialCandy appears twice in the treats array");
// Log the entire treats array for visibility
console.log("Total treats in array:", allTreats.length);
for(uint256 i = 0; i < allTreats.length; i++) {
console.log("Treat at index", i, ":", allTreats[i]);
}
vm.stopPrank();
}

Impact

This creates several issues:

  • Array bloat from duplicate entries

  • Inconsistent state between mapping and array

  • Increased gas costs for array operations

  • Potential confusion for users/interfaces

  • Possible DoS vector through array size manipulation

Tools Used

Forge

Recommendations

Either prevent duplicate names or implement proper name overwrite functionality that maintains data structure consistency.

Updates

Appeal created

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