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

(LOW) `updateVerifier` accepts `address(0)`, letting the owner brick all future claims until they push a new update

snavohburmaa

Location: contracts/src/TreasureHunt.sol:263-269; compare to constructor at contracts/src/TreasureHunt.sol:68

Description

The constructor rejects a zero-address verifier:

if (_verifier == address(0)) revert InvalidVerifier();

But updateVerifier does not reproduce that guard:

function updateVerifier(IVerifier newVerifier) external {
require(paused, "THE_CONTRACT_MUST_BE_PAUSED");
require(msg.sender == owner, "ONLY_OWNER_CAN_UPDATE_VERIFIER");
verifier = newVerifier;
emit VerifierUpdated(address(newVerifier));
}

Risk

Likelihood: Low. Requires an owner mistake.

Impact: Medium. If the owner accidentally passes address(0), every future claim() reverts on the external call to the verifier. The contract is recoverable if owner can call updateVerifier again, but all claims fail and the hunt is paused until the owner updates again .

Proof of Concept

function test_UpdateVerifierAcceptsZeroAddress() public {
vm.prank(owner);
hunt.pause();
​
vm.prank(owner);
hunt.updateVerifier(HonkVerifier(address(0))); // should revert but doesn't
​
assertEq(hunt.getVerifier(), address(0));
}

Run:

forge test --match-test test_UpdateVerifierAcceptsZeroAddress -vv

The test passes updateVerifier(address(0)) is accepted.

Recommended Mitigation

Match the constructor's guard:

function updateVerifier(IVerifier newVerifier) external {
require(paused, "THE_CONTRACT_MUST_BE_PAUSED");
require(msg.sender == owner, "ONLY_OWNER_CAN_UPDATE_VERIFIER");
+ if (address(newVerifier) == address(0)) revert InvalidVerifier();
verifier = newVerifier;
emit VerifierUpdated(address(newVerifier));
}
Updates

Lead Judging Commences

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

no zero-address check in updateVerifier()

The issue is that `updateVerifier()` allows the owner to replace the verifier with an arbitrary address, including `address(0)`, even though the constructor explicitly treats a zero verifier as invalid and reverts with `InvalidVerifier()` during initial deployment. In other words, the contract establishes at deployment time that a null verifier address is not an acceptable configuration, but then fails to preserve that same invariant when the verifier is later updated through the admin recovery path.

Support

FAQs

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

Give us feedback!