SNARKeling Treasure Hunt

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

(MEDIUM) Claimed event emits msg.sender instead of recipient, so every off-chain indexer records the wrong payee

Location: contracts/src/TreasureHunt.sol:44 (declaration), contracts/src/TreasureHunt.sol:107, 111 (emission)

Description

The Claimed event is declared with a parameter named recipient, and every reader will treat the second indexed topic as the address that received the ETH reward. But inside claim() the emission passes msg.sender, which is the address that submitted the transaction not the address that received the ETH.

The declaration (line 44):

event Claimed(bytes32 indexed treasureHash, address indexed recipient);

The claim function pays ETH to recipient (line 107) but emits msg.sender as the second topic (line 111):

function claim(bytes calldata proof, bytes32 treasureHash, address payable recipient) external nonReentrant() {
...
(bool sent, ) = recipient.call{value: REWARD}(""); // ETH actually goes to `recipient`
require(sent, "ETH_TRANSFER_FAILED");
@> emit Claimed(treasureHash, msg.sender); // but event shows the submitter
}

Because the protocol design explicitly forbids recipient == msg.sender at line 86:

if (recipient == address(0) || recipient == address(this) || recipient == owner || recipient == msg.sender) revert InvalidRecipient();

these two addresses are always different. Every Claimed event therefore records a different address from the one that received the ETH. There is no call path where the event is correct.

Risk (M)

Likelihood: High. The bug fires on every successful claim. There is no edge case or special configuration needed to trigger it.

Impact: Medium. No funds are lost on-chain; the ETH transfer itself goes to the correct recipient. The damage is in the off-chain view of the system:

Proof of Concept

The test uses vm.expectEmit with topics (true, true, false, false) to assert that the event's second indexed topic equals attacker (the submitter). The test passes, proving the event really does emit the submitter, while recipient.balance increases by REWARD, proving the ETH went to the actual recipient.

// contracts/test/PoC.t.sol
event Claimed(bytes32 indexed treasureHash, address indexed recipient);
function test_ClaimedEventEmitsWrongRecipient() public {
(bytes memory proof, bytes32 treasureHash, address payable recipient) = _loadFixture();
// Assert: the emitted event will show `attacker` as the "recipient" topic,
// even though the ETH actually goes to `recipient` from the fixture.
vm.expectEmit(true, true, false, false, address(hunt));
emit Claimed(treasureHash, attacker); // wrong on purpose: contract bug
vm.prank(attacker);
hunt.claim(proof, treasureHash, recipient);
// Confirm real recipient got paid - the event is therefore misleading.
assertEq(recipient.balance, 10 ether);
}

Run:

forge test --match-test test_ClaimedEventEmitsWrongRecipient -vv

The test passes, which means the contract's event layout disagrees with reality.

Recommended Mitigation

The simplest fix is to emit the actual recipient:

- emit Claimed(treasureHash, msg.sender);
+ emit Claimed(treasureHash, recipient);
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!