SNARKeling Treasure Hunt
First Flight #59
Beginner FriendlyGameFiFoundry
100 EXP
View results
Previous Next
Submission Details
Severity: low
Valid

onlyOwner modifier declared but never used — all admin functions re-implement the owner check inline

blackburn

Root + Impact

Description

  • The contract defines an onlyOwner modifier but never applies it to any function. Every admin function manually re-implements the same owner check using require(msg.sender == owner, "...") inline. This creates code duplication across 5 functions and wastes deployment gas on a modifier that is never called. The nonReentrant modifier is used correctly — onlyOwner is not.

// @> declared but never applied to any function
modifier onlyOwner() {
require(msg.sender == owner, "ONLY_OWNER");
_;
}
​
// @> same check manually repeated in every admin function instead
function fund() external payable {
require(msg.sender == owner, "ONLY_OWNER_CAN_FUND");
}
function pause() external {
require(msg.sender == owner, "ONLY_OWNER_CAN_PAUSE");
}
function unpause() external {
require(msg.sender == owner, "ONLY_OWNER_CAN_UNPAUSE");
}

Risk

Impact:

  • Deployment gas wasted on dead modifier bytecode

  • Duplicated owner check logic across 5 functions — a future maintenance change could update some but miss others

  • Inconsistent with nonReentrant which IS applied correctly via modifier

Recommended Mitigation

Apply onlyOwner consistently across fund(), pause(), unpause(), withdraw(), and remove the inline require checks.

- function fund() external payable {
- require(msg.sender == owner, "ONLY_OWNER_CAN_FUND");
+ function fund() external payable onlyOwner {
Updates

Lead Judging Commences

s3mvl4d Lead Judge 18 days ago
Submission Judgement Published
Validated
Assigned finding tags:

broken access control in withdraw()

The `withdraw()` function is intended as an owner-only post-hunt recovery function, but the implementation does not actually enforce any ownership check before transferring the full remaining balance to owner. The function only requires that `claimsCount >= MAX_TREASURES` and that the contract balance is nonzero, after which it sends all ETH to the stored owner address regardless of who called the function. Therefore, the access control on the function itself is incomplete because any external account can trigger the withdrawal path once the hunt is considered over.

Support

FAQs

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

Give us feedback!