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

[L-05] Declared custom errors are not used, creating inconsistent revert surface

Author Revealed upon completion

Root + Impact

Description

The contract declares a broad set of custom errors intended for authorization and state checks.

The issue is that many of these declared custom errors are never used, while the runtime checks use string-based require(...) reverts instead. This creates an inconsistent revert interface and defeats the gas-efficiency benefit of custom errors.

// contracts/src/TreasureHunt.sol
error HuntNotOver();
error NoFundsToWithdraw();
error OnlyOwnerCanFund();
error OnlyOwnerCanPause();
error OnlyOwnerCanUnpause();
error TheContractMustBePaused();
error OnlyOwnerCanUpdateVerifier();
error OnlyOwnerCanEmergencyWithdraw();
error InvalidAmount();
​
function withdraw() external {
require(claimsCount >= MAX_TREASURES, "HUNT_NOT_OVER");
require(address(this).balance > 0, "NO_FUNDS_TO_WITHDRAW");
}
​
function fund() external payable {
require(msg.sender==owner, "ONLY_OWNER_CAN_FUND");
}

Risk

Likelihood:

  • The inconsistency is deterministic and affects all executions of the related paths.

  • Integrators relying on ABI-level custom-error decoding cannot use the declared errors for these branches.

Impact:

  • No direct loss of funds.

  • Increased gas cost for revert paths and inconsistent developer/integrator experience due to mixed revert styles.

Severity rationale:

  • Classified as Low because impact is operational/efficiency and interface consistency, not fund safety.

Proof of Concept

Written reproduction flow:

  1. Inspect declared errors in TreasureHunt.sol.

  2. Trigger guarded branches such as withdraw() before hunt completion or fund() by non-owner.

  3. Observe revert data corresponds to Error(string) payloads ("HUNT_NOT_OVER", "ONLY_OWNER_CAN_FUND") instead of declared custom error selectors.

function test_CustomErrorSetDeclaredButStringRevertsUsed() public {
// withdraw() guard uses string revert, not HuntNotOver()
vm.expectRevert(bytes("HUNT_NOT_OVER"));
hunt.withdraw();
​
// fund() guard uses string revert, not OnlyOwnerCanFund()
vm.prank(address(0xBEEF));
vm.expectRevert(bytes("ONLY_OWNER_CAN_FUND"));
hunt.fund{value: 1 ether}();
}

Recommended Mitigation

Use custom errors consistently in guarded functions (withdraw, fund, pause, unpause, updateVerifier, emergencyWithdraw) or remove unused error declarations.

function withdraw() external {
- require(claimsCount >= MAX_TREASURES, "HUNT_NOT_OVER");
- require(address(this).balance > 0, "NO_FUNDS_TO_WITHDRAW");
+ if (claimsCount < MAX_TREASURES) revert HuntNotOver();
+ if (address(this).balance == 0) revert NoFundsToWithdraw();
}
​
function fund() external payable {
- require(msg.sender==owner, "ONLY_OWNER_CAN_FUND");
+ if (msg.sender != owner) revert OnlyOwnerCanFund();
}

Support

FAQs

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

Give us feedback!