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

Replay protection bypass due to incorrect mapping key enables repeated claims and full fund drain

blackburn

Root + Impact

Description

  • The claim() function is intended to mark a treasure as claimed after a valid proof is submitted, preventing the same proof from being used again. The replay guard checks claimed[_treasureHash], where _treasureHash is a private immutable that is never assigned and defaults to bytes32(0). However, the claim is recorded using the function parameter claimed[treasureHash]. Since these keys differ, the check and the write operate on different storage slots, causing the replay protection to never trigger.

// _treasureHash is declared but never assigned in the constructor → always bytes32(0)
bytes32 private immutable _treasureHash;
​
function claim(bytes calldata proof, bytes32 treasureHash, address payable recipient) external nonReentrant() {
// @> always checks claimed[bytes32(0)] — never true, replay protection never fires
if (claimed[_treasureHash]) revert AlreadyClaimed(treasureHash);
...
// @> correctly marks claimed[treasureHash] but this slot is never read by the guard above
_markClaimed(treasureHash);
}

Risk

Likelihood:

  • Any participant who finds a treasure has a valid proof with their address as recipient

  • Replaying requires no additional capital — same calldata submitted multiple times

  • No special access or timing required — works any time the contract is unpaused and funded

Impact:

  • Full contract drain of up to MAX_TREASURES × REWARD using a single valid proof

  • claimsCount hits MAX_TREASURES via replay, permanently locking out all legitimate finders with AllTreasuresClaimed

  • Contract state corrupted — remaining treasure hashes appear unclaimed via isClaimed() but can never be claimed

Proof of Concept

// Assumes attacker has a single valid proof for one treasure
contract MockVerifier is IVerifier {
// Mock Verifying function which always returns true
function verify(bytes calldata, bytes32[] calldata) external pure override returns (bool) {
return true;
}
}
​
//Replay attack by claiming reward with same proof until attacker drains all the fund
function testReplayAttackDrainsContract() public {
bytes memory proof = hex"deadbeef";
bytes32 treasureHash = bytes32(uint256(1)); // Anything non zero
​
vm.startPrank(attacker);
for (uint256 i = 0; i < hunt.MAX_TREASURES(); i++) {
hunt.claim(proof, treasureHash, payable(recipient));
}
vm.stopPrank();
​
assertEq(address(hunt).balance, 0);
assertEq(recipient.balance, 100 ether);
assertEq(hunt.claimsCount(), 10);
}

Recommended Mitigation

- if (claimed[_treasureHash]) revert AlreadyClaimed(treasureHash);
+ if (claimed[treasureHash]) revert AlreadyClaimed(treasureHash);
// Also remove the unused _treasureHash immutable entirely.
Updates

Lead Judging Commences

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

broken double-claim protection

In `claim()`, the guard uses `claimed[_treasureHash]`, where `_treasureHash` is an immutable state variable that is never initialized to the caller-supplied treasure identifier, while the contract later marks `claimed[treasureHash] = true` using the function argument instead. As a result, the duplicate-claim check and the state update are performed against different keys, which means a previously claimed treasure is not actually blocked from being claimed again with the same valid proof and `treasureHash`. This breaks a core invariant of the protocol described in the README, namely, that each treasure can only be redeemed once, and allows one valid treasure/proof pair to be reused to drain rewards repeatedly until either the `MAX_TREASURES` cap or the contract balance is exhausted.

Support

FAQs

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

Give us feedback!