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

`SpookySwap:Treat` overrides, previously created tokens with the same name are overridden.

edmpulasky

Summary

SpookySwap:addTreat with the same name, overrides previously added treats, affecting treatList[_treatName] access, which queries the latest.

Vulnerability Details

No overrides protection, users are supposed to TrickOrTreat over specific items tagged by _name and eventually being spooked by double/half/same price. Though ahead of his TX on-chain might exists a one which overrides with newer prices affecting dependent functions of treatList[_treatName].

function addTreat(string memory _name, uint256 _rate, string memory _metadataURI) public onlyOwner {
@> treatList[_name] = Treat(_name, _rate, _metadataURI);
treatNames.push(_name);
emit TreatAdded(_name, _rate, _metadataURI);
/*
...impl...
*/
}
​
function trickOrTreat(string memory _treatName) public payable nonReentrant {
@> Treat memory treat = treatList[_treatName];
require(treat.cost > 0, "Treat cost not set.");
​
/*
...impl...
*/
}
function resolveTrick(uint256 tokenId) public payable nonReentrant {
require(pendingNFTs[tokenId] == msg.sender, "Not authorized to complete purchase");
​
string memory treatName = tokenIdToTreatName[tokenId];
@> Treat memory treat = treatList[treatName];
​
/*
...impl...
*/
}
​

Impact

Overrides cause invalid queries for treatList[_treatName] which makes SpookySwap:setTreatCost to update the cost of the latest treat, besides SpookySwap:trickOrTreat mints the latest treat and SpookySwap:resolveTrick even being tied to a tokenId it still queries a treat by _name.

Tools Used

  • Manual Review

  • Foundry Testing tool

function testTreatOverrides() public {
protocol.addTreat("candy", 0.1 ether, "uri1");
protocol.addTreat("candy", 0.2 ether, "uri2");
uint256 tokenId3 = protocol.nextTokenId();
protocol.addTreat("candy", 0.3 ether, "uri3");
//confirm 3 treats were added
uint256 treats = (protocol.getTreats()).length;
assertEq(treats, 3);
//update latest "candy token"
protocol.setTreatCost("candy", 0.5 ether);
//token 1 and 2 are not accesible by no means
(string memory name, uint256 cost,) = protocol.treatList("candy");
​
assert(cost == 0.5 ether && Strings.equal(name, "candy"));
//TrickOrTreat are applied to latest candy
vm.prank(user);
protocol.trickOrTreat{ value: 1 ether }("candy");
​
string memory uri = protocol.tokenURI(tokenId3);
assert(Strings.equal(uri, "uri3"));
}

Recommendations

Implement a mapping that indicates which treat name has been already added to treatList[_treatName], check name uniqueness before adding

  • mapping(string => bool) public exists

contract SpookySwap is ERC721URIStorage, Ownable(msg.sender), ReentrancyGuard {
+ mapping(string => bool) public exists
/* impl */
function addTreat(string memory _name, uint256 _rate, string memory _metadataURI) public onlyOwner {
+ require(!exists[_name], "Treat already added");
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.