SNARKeling Treasure Hunt

First Flight #59
Beginner FriendlyGameFiFoundry
100 EXP
View results
Submission Details
Severity: low
Valid

`Claimed` event emits `msg.sender` under a parameter declared as `recipient`, misleading off-chain consumers

Description

  • The Claimed event is declared with address indexed recipient as its second topic, intended for off-chain consumers (indexers, dashboards, notification services) to filter by the address that actually received the payout.

  • At the emit site msg.sender is passed in the recipient slot. Since claim() enforces recipient != msg.sender earlier in the same function, the emitted topic is guaranteed to be the wrong address on every successful claim; the topic labelled recipient in the ABI never contains the real recipient.

// contracts/src/TreasureHunt.sol
event Claimed(bytes32 indexed treasureHash, address indexed recipient); // @> second topic is named "recipient"
function claim(bytes calldata proof, bytes32 treasureHash, address payable recipient) external nonReentrant() {
...
@> if (recipient == address(0) || recipient == address(this) || recipient == owner || recipient == msg.sender) revert InvalidRecipient();
...
(bool sent, ) = recipient.call{value: REWARD}("");
require(sent, "ETH_TRANSFER_FAILED");
@> emit Claimed(treasureHash, msg.sender); // @> msg.sender, not recipient — always the wrong value
}

Risk

Likelihood:

  • Fires on every successful claim because the emit site is hardcoded and the recipient-is-caller case is explicitly blocked on line 86.

  • Any off-chain tool that filters Claimed logs by recipient address (e.g. a participant's own UI saying "your claim landed") will silently return empty or mismatched results.

Impact:

  • No funds at risk; the ETH transfer itself goes to the correct address.

  • Off-chain analytics and user-facing notifications keyed on the recipient topic are systematically corrupted. Leaderboards, explorers, and cron jobs that reconcile on-chain payouts against event logs will disagree with the chain state.

  • Debuggability is harmed: event filters keyed on the real recipient return zero matches, complicating incident response and post-hunt accounting.

Proof of Concept

Inspecting any successful claim() call surfaces the mismatch. The Transfer-like ETH movement on line 107 targets recipient, while the Claimed log emitted on line 111 records msg.sender in the second indexed topic. Because line 86 rejects the case where recipient == msg.sender, the two are never the same address in any valid execution path.

// Foundry sketch — demonstrates the indexed "recipient" topic is not the payout address
function test_claimedEventRecipientTopicIsCaller() public {
// alice is the claimant, bob is the payout recipient
(bytes memory proof, bytes32 hash) = _makeProof(secret, bob);
vm.recordLogs();
vm.prank(alice);
hunt.claim(proof, hash, payable(bob));
Vm.Log[] memory logs = vm.getRecordedLogs();
// Claimed(bytes32 indexed, address indexed) -> topic[2] is the "recipient" slot
address loggedRecipient = address(uint160(uint256(logs[0].topics[2])));
assertEq(bob.balance, 10 ether); // the real payout recipient
assertEq(loggedRecipient, alice); // but the log records the caller instead
assertTrue(loggedRecipient != bob); // mismatch guaranteed by claim()'s own preconditions
}

Recommended Mitigation

Emit recipient — the parameter the payout is sent to — so the log matches the ETH transfer and the ABI's topic name. If capturing both the claimer and the payout recipient is intentional, widen the event to three indexed parameters rather than repurposing an existing one.

// Fix A: emit the correct parameter
- emit Claimed(treasureHash, msg.sender);
+ emit Claimed(treasureHash, recipient);
// Fix B (if both claimer and recipient are desired): extend the event
- event Claimed(bytes32 indexed treasureHash, address indexed recipient);
+ event Claimed(bytes32 indexed treasureHash, address indexed recipient, address indexed claimer);
...
- emit Claimed(treasureHash, msg.sender);
+ emit Claimed(treasureHash, recipient, msg.sender);
Updates

Lead Judging Commences

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

incorrect event parameter

The event is declared as event `Claimed(bytes32 indexed treasureHash, address indexed recipient);`, which clearly indicates that the second indexed field is meant to represent the reward recipient, but `claim()` emits `Claimed(treasureHash, msg.sender)` instead of `Claimed(treasureHash, recipient)`, even though the ETH transfer is sent to recipient and the proof itself is constructed around the public inputs (treasureHash, recipient). As a standalone finding, this is appropriately low severity because it is fundamentally an event/accounting inconsistency rather than a direct loss-of-funds issue: the core state transition and payout still follow the intended recipient, but off-chain consumers reading the event log will observe incorrect metadata about who was associated with the claim.

Support

FAQs

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

Give us feedback!