Submission Details
Severity:
The `TrickOrTreat::addTreat` function allows overwritten of an existing treat in the `treatList`
Summary
If a treat with the same `_name` already exists, the `addTreat` function will overwrite it without warning. This can lead to issues such as losing critical data or changing a treat’s information unexpectedly.
```javascript
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);
}
```
A malicious user (or even the contract owner, unintentionally) could add a treat with an existing name, thereby replacing the existing treat's data, such as the rate or metadataURI, which could lead to incorrect pricing, confusion, or loss of important treat details.
Vulnerability Details
An initial treat was added with the name Chocolate with a cost of 3 ether, then a new treat was added with the same name and a cost of 4 ether which overwrite the previous treat.
which now set a new Treat.
Here is a proof of code to that effects:
<details>
<summary>Here is the test code</summary>
```javascript
function testAddTreat() public {
spookySwap.addTreat("Chocolate", 3 ether, "https://example.com/chocolate.json");
(string memory name, uint256 cost, string memory uri, ) = spookySwap.treatList("Chocolate");
assertEq(name, "Chocolate");
assertEq(cost, 3 ether);
assertEq(uri, "https://example.com/chocolate.json");
console.log("Initial Treat Added: ", name);
console.log("Cost: ", cost);
console.log("URI: ", uri);
spookySwap.addTreat("Chocolate", 4 ether, "https://example.com/chocolate.json");
(name, cost, uri, ) = spookySwap.treatList("Chocolate");
console.log("New Treat Added: ", name);
console.log("Cost: ", cost);
console.log("URI: ", uri);
}
```
</details>
Impact
An Contract owner can add a treat with the same name as an existing treat, and the existing treat will be overwritten which could lead to incorrect pricing, confusion, or loss of important treat details.
Tools Used
Manual Review
Reccommendation
Adding a check to prevent overwriting of an existing treat in the list. By this you can ensure a treat with a unique name is added by checking whether it already exists before adding it to treatList.
You first add a boolean to the Treat struct to check if the treat already exists, then you can check if the treat exists before adding it to the list.
```diff
struct Treat{
string name;
uint256 cost; // Cost in ETH (in wei) to get one treat
string metadataURI; // URI for the NFT metadata
+ bool exists;
}
```
```diff
function addTreat(string memory _name, uint256 _rate, string memory _metadataURI) public onlyOwner {
+ require(!treatList[_name].exists, "Treat already exists");
+ treatList[_name] = Treat(_name, _rate, _metadataURI, true);
- treatList[_name] = Treat(_name, _rate, _metadataURI);
treatNames.push(_name);
emit TreatAdded(_name, _rate, _metadataURI);
}
```
Updates
Appeal created
bube 7 months ago
Submission Judgement Published 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.
deeney
7 months ago
bube
7 months ago
bube 7 months ago
Submission Judgement Published 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.
Rewards breakdown
nSLOC
109This contest has a flat reward structure with the following payouts:
100 EXP
20 EXP
2 EXP
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.