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

Zero-Cost Treats Not Properly Handled in SpookySwap

bshyuunn

01. Relevant GitHub Links

02. Summary

The smart contract does not handle treats with a zero-cost properly, which leads to critical functionality issues, including setting and purchasing treats.

03. Vulnerability Details

The SpookySwap contract contains a vulnerability where treats with zero cost are added but are not handled correctly when attempting to modify or purchase them. Specifically, the contract lacks adequate checks to prevent zero-cost treats from being added, and subsequently, the functions that modify or allow for purchase of such treats revert as expected but could create confusion and disrupt the intended operations of the contract.

03. Impact

  • Inability to Update Treat Cost: The owner cannot update the cost of a treat once it has been set to zero, as it triggers a revert condition, rendering the treat configuration immutable under these circumstances.

  • Impossible to Purchase Zero-Cost Treats: Users are prevented from purchasing treats that were mistakenly added with a zero cost, which can limit the intended functionality and lead to unintended transaction failures.

04. Proof of Concept

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.24;
​
import {Test, console} from "forge-std/Test.sol";
import {SpookySwap} from "../src/TrickOrTreat.sol";
​
contract TrickOrTreatTest is Test {
SpookySwap spookyswapInstance;
​
address owner;
address user;
​
function setUp() public {
owner = vm.addr(0x1);
user = vm.addr(0x2);
​
vm.label(owner, "Owner");
vm.deal(owner, 100 ether);
​
vm.label(user, "User");
vm.deal(user, 100 ether);
​
SpookySwap.Treat[] memory TreatList;
​
vm.prank(owner);
spookyswapInstance = new SpookySwap(TreatList);
}
​
function test_poc_NobodyHandle0CostTreat() public {
// add 0 cost treat
vm.prank(owner);
spookyswapInstance.addTreat("0CostTreat", 0, "test");
​
// owner cat't change 0CostTreat's cost
vm.prank(owner);
vm.expectRevert("Treat must cost something.");
spookyswapInstance.setTreatCost("0CostTreat", 1 ether);
​
// nobody can buy 0CostTreat
vm.prank(user);
vm.expectRevert("Treat cost not set.");
spookyswapInstance.trickOrTreat{value: 1 ether}("0CostTreat");
}
}
$ forge test --mt test_poc_NobodyHandle0CostTreat -vvvv
[⠊] Compiling...
[⠊] Compiling 2 files with Solc 0.8.27
[⠒] Solc 0.8.27 finished in 1.97s
Compiler run successful!
​
Ran 1 test for test/test.t.sol:TrickOrTreatTest
[PASS] test_poc_NobodyHandle0CostTreat() (gas: 130020)
Traces:
[130020] TrickOrTreatTest::test_poc_NobodyHandle0CostTreat()
├─ [0] VM::prank(Owner: [0x7E5F4552091A69125d5DfCb7b8C2659029395Bdf])
│ └─ ← [Return]
├─ [98752] SpookySwap::addTreat("0CostTreat", 0, "test")
│ ├─ emit TreatAdded(name: "0CostTreat", cost: 0, metadataURI: "test")
│ └─ ← [Stop]
├─ [0] VM::prank(Owner: [0x7E5F4552091A69125d5DfCb7b8C2659029395Bdf])
│ └─ ← [Return]
├─ [0] VM::expectRevert(Treat must cost something.)
│ └─ ← [Return]
├─ [1393] SpookySwap::setTreatCost("0CostTreat", 1000000000000000000 [1e18])
│ └─ ← [Revert] revert: Treat must cost something.
├─ [0] VM::prank(User: [0x2B5AD5c4795c026514f8317c7a215E218DcCD6cF])
│ └─ ← [Return]
├─ [0] VM::expectRevert(Treat cost not set.)
│ └─ ← [Return]
├─ [7841] SpookySwap::trickOrTreat{value: 1000000000000000000}("0CostTreat")
│ └─ ← [Revert] revert: Treat cost not set.
└─ ← [Stop]
​
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 11.26ms (1.10ms CPU time)
​
Ran 1 test suite in 785.64ms (11.26ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)

05. Tools Used

Manual Code Review and Foundry

06. Recommended Mitigation

  1. Prevent Zero-Cost Treats from Being Added: Add a check in the addTreat function to ensure that treats cannot be added with a zero-cost.

require(cost > 0, "Treat must have a non-zero cost.");

  1. Improve Error Handling for Cost-Related Functions: Ensure all functions that interact with treat cost validate the cost to ensure that zero-cost treats are appropriately handled and cannot cause unintended reverts.

Updates

Appeal created

bube Lead Judge 9 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

[invalid] Zero treat cost

The cost of the treat is set only by the owner (in the constructor, in addTreat and in setTreatCost). That means the cost of the treat will always be greater than zero.

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.