SNARKeling Treasure Hunt

Description

• claim() must mark claimed[treasureHash] = true after a successful proof so the same treasureHash cannot be redeemed twice.

• The guard reads the contract's immutable _treasureHash, which is never assigned in the constructor and is therefore permanently bytes32(0). _markClaimed writes under the parameter key, so claimed[0x0] is never set and the check is a permanent no-op. A single valid proof can be replayed until claimsCount == MAX_TREASURES, paying 10 × REWARD = 100 ETH.

Risk

Likelihood:

• Triggers on the happy path — any valid proof submitted (or observed in the mempool) is re-broadcastable as-is, with no privileged role required.

• The project's own testClaimDoubleSpendReverts has vm.expectRevert() commented out and still passes, confirming the replay.

Impact:

• Up to 100 ETH drained to the single recipient bound inside the replayed proof — a 10× payout escalation.

• claimsCount saturates at MAX_TREASURES, bricking the hunt for the 9 remaining finders with AllTreasuresClaimed.

Proof of Concept

The test submits the same (proof, treasureHash, recipient) tuple 10 times. Each iteration passes the guard because it checks claimed[bytes32(0)] instead of claimed[treasureHash], so the proof is accepted over and over until MAX_TREASURES is exhausted — draining the pool to the proof's recipient and permanently blocking every other finder.

Recommended Mitigation

The root cause is a single-identifier typo compounded by a dead storage slot that made the typo type-check. The fix has two parts — the guard must read the caller-supplied parameter, and the unused immutable must be removed so a future refactor cannot silently reintroduce the same bug. _markClaimed already writes under the parameter key, so no storage migration or redeploy sequencing is needed beyond pushing the patched bytecode.

1. Delete the uninitialised immutable. It is never assigned in the constructor and never read anywhere except in the broken guard, so removing it eliminates the only symbol a future author could mistakenly reach for:

2. Check the function parameter in the replay guard. This realigns the read with the write on line 104, so every accepted proof consumes its own slot in the claimed mapping and a replay of the same treasureHash reverts with AlreadyClaimed(treasureHash) on the second attempt:

Post-fix behaviour. The PoC above reverts on its second iteration with AlreadyClaimed(treasureHash); the contract retains the other 90 ETH; each of the remaining nine treasures stays claimable exactly once by its legitimate finder; and withdraw() can be reached honestly once all ten slots are consumed. The existing testClaimDoubleSpendReverts should also be updated — its vm.expectRevert() is currently commented out, so it silently passes under the bug and will continue to pass under the fix unless the assertion is restored to vm.expectRevert(abi.encodeWithSelector(TreasureHunt.AlreadyClaimed.selector, treasureHash)).