SNARKeling Treasure Hunt

First Flight #59
Beginner FriendlyGameFiFoundry
100 EXP
Submission Details
Impact: medium
Likelihood: high

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

Author Revealed upon completion

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

Support

FAQs

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

Give us feedback!