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

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

Author Revealed upon completion

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));
}

Support

FAQs

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

Give us feedback!