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

Lack of Input Validation for Treat Names

abhishekthakur

Summary

  • Root Cause: No validation for empty strings or duplicate treat names

  • Impact: Potential contract state confusion and unusable treats

Vulnerability Details

https://github.com/Cyfrin/2024-10-trick-or-treat/blob/main/src/TrickOrTreat.sol#L37-L41

function addTreat(string memory _name, uint256 _rate, string memory _metadataURI) public onlyOwner {
/// @audit: no check for empty strings or duplicate values
treatList[_name] = Treat(_name, _rate, _metadataURI);
treatNames.push(_name);
emit TreatAdded(_name, _rate, _metadataURI);
}

The addTreat function doesn't validate that treat names are non-empty strings and doesn't prevent duplicate treat names. This could lead to confusion in the contract state and potential issues with treat identification.

If a treat is already exists, it just overrides that with new values.

POC

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.24;
import "forge-std/Test.sol";
import "../src/SpookySwap.sol";
​
contract SpookySwapTest is Test {
SpookySwap spookySwap;
function setUp() public {
SpookySwap.Treat[] memory treats = new SpookySwap.Treat[]();
spookySwap = new SpookySwap(treats);
}
function testEmptyTreatName() public {
// Can add treat with empty name
spookySwap.addTreat("", 1 ether, "uri");
// Can add duplicate treat names
spookySwap.addTreat("Candy", 1 ether, "uri1");
spookySwap.addTreat("Candy", 2 ether, "uri2");
}
}

Impact

  • Confusion in treat identification

  • Potential overrding of existing values that can affect users

Tools Used

Manual Review

Recommendations

Here is the recomendation that can be implemented to avoid these issues -

function addTreat(string memory _name, uint256 _rate, string memory _metadataURI) public onlyOwner {
require(bytes(_name).length > 0, "Treat name cannot be empty");
require(treatList[_name].cost == 0, "Treat name already exists");
require(bytes(_metadataURI).length > 0, "URI cannot be empty");
treatList[_name] = Treat(_name, _rate, _metadataURI);
treatNames.push(_name);
emit TreatAdded(_name, _rate, _metadataURI);
}
Updates

Appeal created

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