SNARKeling Treasure Hunt

Root + Impact

Description

The `Claimed` event in `TreasureHunt.sol` declares a `recipient` field (line 44) but the emit statement on line 111 passes `msg.sender` instead of the actual `recipient` parameter that received the ETH:

```solidity
// Event declaration (line 44)
event Claimed(bytes32 indexed treasureHash, address indexed recipient);

// Event emission (line 111, inside claim())
emit Claimed(treasureHash, msg.sender);
```

Because `claim()` explicitly forbids `recipient == msg.sender` on line 86 (`if (recipient == address(0) || ... || recipient == msg.sender) revert InvalidRecipient();`), the `recipient` and `msg.sender` are always different addresses on every successful claim. The `Claimed` event therefore indexes the transaction CALLER into the `recipient` topic - not the address that actually received the 10 ETH reward.

Risk

Likelihood: High - the mismatch occurs on every single successful `claim()` call; there is no code path where `msg.sender == recipient` is allowed.

Impact

• Off-chain indexers, block-explorer UIs, and dashboards subscribed to `Claimed(treasureHash, recipient)` to track "who won which treasure" will attribute every reward to the transaction submitter instead of the actual payout destination.

• Graph-node subgraphs and Dune queries filtering by the indexed `recipient` field miss legitimate winners when a user submits `claim()` via a relayer, meta-transaction, or any wallet-separate-from-recipient flow - "show all claims where I am the recipient" returns false negatives.

• Forensic investigations after a potential exploit cannot rely on the `Claimed` event to reconstruct fund flow; auditors must instead cross-reference internal ETH transfers or trace the `recipient` argument from calldata.

• Combined with the broken double-claim guard (separate finding), an attacker who drains the pool via replay would emit 10 `Claimed` events indexed by their own EOA (`msg.sender`), making the attack look like 10 distinct victim-initiated claims in naive event-based monitoring.

Proof of Concept

Exploit steps:

1. Participant Alice controls two addresses: `Alice_EOA` (submits transactions) and `Alice_Cold` (hardware wallet used as the ZK proof's `recipient`).

2. Alice generates a valid proof `P` for `(treasureHash = H, recipient = Alice_Cold)` and calls `claim(P, H, Alice_Cold)` from `Alice_EOA`.

3. Inside `claim()`, line 86 passes (recipient != caller), line 107 transfers 10 ETH to `Alice_Cold`, and line 111 emits `Claimed(H, Alice_EOA)`.

4. A subgraph indexing "all claims by recipient" with query `WHERE recipient = Alice_Cold` returns zero results; the event's `recipient` field indexes `Alice_EOA`. Alice's cold wallet appears never to have won anything despite receiving 10 ETH.

A minimal Foundry test demonstrating the mismatch:

```solidity
function test_EventEmitsWrongRecipient() public {
vm.recordLogs();
vm.prank(participant); // msg.sender = participant
hunt.claim(PROOF, TREASURE_HASH, recipient); // actual payout goes to recipient

Vm.Log[] memory logs = vm.getRecordedLogs();
// topic[2] = indexed recipient param
address indexedRecipient = address(uint160(uint256(logs[logs.length - 1].topics[2])));

assertEq(recipient.balance, 10 ether);             // ETH landed at recipient (correct)
assertEq(indexedRecipient, participant);           // event indexes msg.sender, not recipient
assertTrue(indexedRecipient != recipient);         // subgraphs querying by recipient miss this

}
```

Recommended Mitigation

Emit the `recipient` parameter that actually received the ETH, matching the event declaration:

```diff

•    emit Claimed(treasureHash, msg.sender);
  

•    emit Claimed(treasureHash, recipient);
  

```

This restores the documented contract of the event so subgraphs and indexers correctly attribute each 10 ETH payout to its destination. The caller is already implicitly available as `tx.from` in the log metadata, so no information is lost; downstream tooling that wants the submitter can read that field instead.

For additional observability, augment the event to include both parties explicitly:

```solidity
event Claimed(bytes32 indexed treasureHash, address indexed recipient, address indexed caller);
```

Adding `caller` as a third indexed field lets downstream consumers query by either "who won" or "who submitted" without ambiguity, and directly prevents a future copy-paste accident of the same class from silently breaking analytics.