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

Protocol did not check the treat name before add them into treatList

0xdennnnny

Summary

When adding treats, no matter in constructor or addTreat function, protocol did not check the param name.
This might make some user get confused.
When they try to execute trickOrTreat function. They might not get what they want.
For example, there are two same-name treat in treatList : ['treat1', 'treat2', 'treat2', 'treat3'].
But first one cost only 3 ether, and second one cost 10 ether.
A user just happen to know treat called : treat2 . So he decide to execute trickOrTreat function
And what he/she expects is cost 3 ether in normal situation(6 ether when tricky, and 1.5 ether when treaty).
But actually the treat2 he/she calls, it's cost 10 ether.
So 5 ether in treaty situation and 20 ether in tricky situation.
Totally confused the user.

Vulnerability Details

try to add test below.
Find out it might confused if same treat with different cost happened.

POC

​
function test_addTreatInSameName() public {
string memory newName = "newTreat";
uint256 newCost = 100;
string memory newUri = "new_uri_100";
​
string memory newName_sameName = "newTreat";
uint256 newCost_sameName = 200;
string memory newUri_sameName = "new_uri_200";
​
vm.prank(deployer);
swap.addTreat(newName, newCost, newUri);
vm.prank(deployer);
swap.addTreat(newName_sameName, newCost_sameName, newUri_sameName);
​
(, uint256 expectedCost, ) = swap.treatList(newName);
​
assert(expectedCost == newCost_sameName);
// it get the latest treat's content!!!!
// it'll confuse user when there are same name treat inside the treatlist
}
​

Impact

If treatList includes same-name treat more than one.
That's say, maybe 5 more same-name treat but different cost.
It will confused user.

Tools Used

Manually

Recommendations

Consider check treatList[name] is existed or not.
Or if same-name is acceptable, at least they should have same cost and metaURI.

​
constructor(Treat[] memory treats) ERC721("SpookyTreats", "SPKY") {
nextTokenId = 1;
​
for (uint256 i = 0; i < treats.length; i++) {
+ if(treatList[treats[i].name]) revert SpookySwap__SameNameTreatExisted();
addTreat(treats[i].name, treats[i].cost, treats[i].metadataURI);
}
}
​
function addTreat(
string memory _name,
uint256 _rate,
string memory _metadataURI
) public onlyOwner {
+ if(treatList[_name]) revert SpookySwap__SameNameTreatExisted();
treatList[_name] = Treat(_name, _rate, _metadataURI);
treatNames.push(_name);
emit TreatAdded(_name, _rate, _metadataURI);
}
​
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.