Puppy Raffle

Root + Impact

Description

refund() violates the checks-effects-interactions (CEI) pattern: it sends ETH to msg.sender via sendValue() BEFORE updating contract state. The state update (players[playerIndex] = address(0)) only executes after the external call returns.

If the player is a smart contract, its receive() function runs during the sendValue() call — at that moment players[playerIndex] still holds the attacker's address, so a reentrant call to refund(playerIndex) passes both require checks again:

Each reentrant call sends another entranceFee before any state is written, so one ticket can be refunded repeatedly until the contract balance is drained.

Risk

Likelihood: High — anyone can enter as a contract; the attack needs no privileged access, no timing assumptions, and no cooperation from other users. The only cost is one entrance fee.

Impact: High — the attacker drains the entire raffle balance, stealing every honest player's entrance fee. Afterwards selectWinner() cannot cover the prize pool, so the raffle is permanently broken.

Proof of Concept

The Foundry test below demonstrates the full attack flow:

Test assertions and measured result (forge test --match-test test_PoC_H1_ReentrancyDrain):

Recommended Mitigation

Apply checks-effects-interactions: zero the player slot BEFORE the external call. On any reentrant attempt, players[playerIndex] is already address(0), so the playerAddress != address(0) check reverts and the attack fails at the first reentry. Adding OpenZeppelin's ReentrancyGuard gives defense in depth against future code changes that might reintroduce the pattern.