SNARKeling Treasure Hunt
First Flight #59
Beginner FriendlyGameFiFoundry
100 EXP
View all submissions
Previous Next
Submission Details
Impact: low
Likelihood: medium

(LOW) Custom errors are declared but most reverts use string `require` messages, so the declared errors are dead code.

Author Revealed upon completion

Location: contracts/src/TreasureHunt.sol:7-25 (declarations)

Description

TreasureHunt.sol:7-25 declares custom errors such as OnlyOwnerCanFund, OnlyOwnerCanPause, NoFundsToWithdraw, HuntNotOver, InvalidAmount. They are never used. The contract reverts with strings instead:

require(msg.sender==owner, "ONLY_OWNER_CAN_FUND");
require(msg.value > 0, "NO_ETH_SENT");
require(claimsCount >= MAX_TREASURES, "HUNT_NOT_OVER");
require(balance > 0, "NO_FUNDS_TO_WITHDRAW");

Risk

Likelihood: Medium every admin path and every revert uses strings instead of the declared custom errors.

Impact: Low. Higher gas on revert, larger bytecode, noisier ABI, tooling that parses error selectors cannot match the declared ABI.

Proof of Concept

function test_FundUsesStringNotCustomError() public {
// Non-owner call reverts with a STRING, not the declared OnlyOwnerCanFund selector.
vm.expectRevert(bytes("ONLY_OWNER_CAN_FUND"));
hunt.fund{value: 1 ether}();
}

Run:

forge test --match-test test_FundUsesStringNotCustomError -vv

The test passes, confirming the custom error OnlyOwnerCanFund is declared but never selected.

Recommended Mitigation

Switch every require(..., "STRING") to if (!cond) revert CustomError(); using the already declared errors. Example:

- require(msg.sender==owner, "ONLY_OWNER_CAN_FUND");
+ if (msg.sender != owner) revert OnlyOwnerCanFund();
​
- require(claimsCount >= MAX_TREASURES, "HUNT_NOT_OVER");
+ if (claimsCount < MAX_TREASURES) revert HuntNotOver();

Support

FAQs

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

Give us feedback!