SNARKeling Treasure Hunt

Root + Impact

Description

The constructor of `TreasureHunt.sol` validates that the verifier address is non-zero on line 68:

```solidity
constructor(address _verifier) payable {
if (_verifier == address(0)) revert InvalidVerifier();
// ...
}
```

But `updateVerifier()` on lines 263-269 - the only other path that writes to the `verifier` storage slot - is missing the same validation:

```solidity
function updateVerifier(IVerifier newVerifier) external {
require(paused, "THE_CONTRACT_MUST_BE_PAUSED");
require(msg.sender == owner, "ONLY_OWNER_CAN_UPDATE_VERIFIER");
verifier = newVerifier; // no zero-address check
emit VerifierUpdated(address(newVerifier));
}
```

If the owner calls `updateVerifier(IVerifier(address(0)))` - either accidentally through a fat-fingered transaction, a bad multisig proposal, or a UI bug - `verifier` is silently set to the zero address. Every subsequent `claim()` then reverts on line 100 (`bool ok = verifier.verify(proof, publicInputs);`) because the call to `address(0)` returns empty data that cannot be decoded as `bool` - the contract's core redemption path is bricked until the owner notices the misconfiguration and issues another `updateVerifier` call with the correct address.

Risk

Likelihood: Low - requires an owner mistake (fat-finger, buggy UI, bad multisig signing), but the mistake is easy to make because the input field accepts any address including `0x0`.

Impact

• All in-flight `claim()` attempts start reverting immediately after the erroneous update; participants who physically found treasures cannot redeem them until the owner re-fixes the verifier.

• Because `updateVerifier` requires the contract to be paused, users trying to claim during the misconfiguration window see `ContractPaused()` followed by a verification failure once unpaused, obscuring the real cause.

• The contract emits `VerifierUpdated(0x0)` which, if indexed by a monitoring system, looks syntactically valid - monitoring operators may not flag a "successful" admin action as broken.

• The inconsistency with the constructor signals intent: the project KNEW zero-address verifiers are unacceptable (hence the constructor check), but that intent was not carried into the update path.

Proof of Concept

Exploit steps (the "exploit" is an accidental owner mis-operation):

1. Contract deployed normally with valid verifier; participants begin claiming treasures.

2. Owner pauses the contract (e.g., to investigate a reported issue, per the stated use case "In case of a bug, allow the owner to update the verifier address").

3. Owner calls `updateVerifier(IVerifier(address(0)))` by mistake - a front-end form submits an empty address, a multisig template has a default zero-address parameter, or a transcription error during manual address entry.

4. `verifier` storage slot now holds `address(0)`. The `VerifierUpdated(0x0)` event fires with no warning.

5. Owner calls `unpause()` believing the fix is complete.

6. Alice attempts `claim(proof, treasureHash, recipient)`. Execution reaches line 100: `bool ok = verifier.verify(proof, publicInputs);`. The call to `address(0).verify(...)` returns empty return data, which cannot be abi-decoded as `bool`, causing an EVM-level revert without a clear error message.

7. The hunt is bricked until the owner notices and submits a second `updateVerifier` call with the real verifier address - potentially hours or days of downtime.

Recommended Mitigation

Add the same zero-address check from the constructor:

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

This reuses the existing `InvalidVerifier()` custom error (line 15) and produces a clear revert reason when an accidental zero-address update is attempted, making the failure mode visible at transaction simulation time instead of silently bricking the contract.

For additional robustness, consider calling the new verifier with a known-invalid proof and requiring a specific revert signature - this proves the target actually implements the `IVerifier` interface before committing the storage change, catching both zero addresses and addresses pointing to random non-verifier contracts.