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

`Claimed` event indexes `msg.sender`, not the payout `recipient`

virgilbb

Root + Impact

Description

  • Normal behavior: Events should match their signatures and NatSpec so indexers can rely on indexed fields. The Claimed event’s second parameter is named recipient, so listeners expect the payout address.

  • Problem: The contract pays recipient but emits msg.sender as the second indexed argument. The contract also forbids recipient == msg.sender (InvalidRecipient), so the indexed address is never the payout address. Off-chain systems that filter or display “recipient” from logs will show the claimer, not who received the ETH.

// @> event documents "recipient" as second indexed topic
event Claimed(bytes32 indexed treasureHash, address indexed recipient);
​
// @> payout goes to recipient
(bool sent, ) = recipient.call{value: REWARD}("");
​
// @> second topic is msg.sender, not recipient
emit Claimed(treasureHash, msg.sender);
​

Risk

Likelihood:

  • Every successful claim produces logs with this shape.

  • Explorers, subgraphs, and accounting pipelines often index Claimed without re-reading calldata.

Impact:

  • Misattribution of who was paid; reconciliation errors and misleading UI.

  • No direct on-chain fund loss; integrity and operational risk dominate.

Proof of Concept

Explanation: Compare the declared event, the transfer target, and the emit. The second indexed topic is always msg.sender (the participant), while ETH is sent to recipient (a different address whenever the participant uses a separate payout address).

Supporting code (contract, contest repo):

// contracts/src/TreasureHunt.sol
if (recipient == address(0) || recipient == address(this) || recipient == owner || recipient == msg.sender) revert InvalidRecipient();
// ...
(bool sent, ) = recipient.call{value: REWARD}("");
emit Claimed(treasureHash, msg.sender);
​

Supporting check (run any passing claim test and decode logs):

forge test --match-test testClaimHappyPath -vvvv
​

Inspect Claimed topics: topic2 matches msg.sender, not the recipient argument passed to claim.

Recommended Mitigation

Explanation: Align the event payload with its name and NatSpec. Emit the actual payee (recipient) as the second indexed field if that is what integrators should track. If both claimer and payee matter, add a separate indexed field or a second event; update frontend and indexer schemas accordingly.

- emit Claimed(treasureHash, msg.sender);
+ emit Claimed(treasureHash, recipient);
​

Optional: rename the parameter for clarity if you also emit msg.sender elsewhere, for example event Claimed(bytes32 indexed treasureHash, address indexed payee, address indexed claimer) if both are required.

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!