Root + Impact

Description

• withdrawFees() at PuppyRaffle.sol:158 uses strict equality to verify no players are active:

• An attacker can permanently break this check by force-sending ETH to the contract via selfdestruct. The selfdestruct opcode bypasses all receive() and fallback() functions, forcefully increasing the contract's balance. Once balance > totalFees, the strict equality never holds again, and all accumulated protocol fees are permanently locked.

• Attack cost: 1 wei + gas (negligible).

Risk

Likelihood:

• Trivially exploitable — any address can deploy and selfdestruct a contract with 1 wei directed at PuppyRaffle

• No permissions required, no timing constraints

Impact:

• All accumulated protocol fees locked permanently in the contract

• The feeAddress (protocol owner) can never withdraw fees

• Combined with H-003 (integer overflow), this creates a doubly-locked state

Proof of Concept

How the attack works:

1. The protocol completes a raffle round via selectWinner(), accumulating fees in totalFees — at this point address(this).balance == uint256(totalFees) holds

2. An attacker deploys a minimal contract with a selfdestruct(payable(puppyRaffleAddress)) function, funded with just 1 wei

3. The selfdestruct opcode forcefully sends the 1 wei to PuppyRaffle, bypassing all receive()/fallback() logic — the contract's balance increases but totalFees does not

4. Now address(this).balance > uint256(totalFees), and the strict == check in withdrawFees() permanently fails, locking all protocol fees

PoC code:

Expected outcome: After force-sending 1 wei via selfdestruct, the strict equality address(this).balance == uint256(totalFees) never holds again, permanently locking all accumulated protocol fees in the contract.

Recommended Mitigation

The root cause is that withdrawFees() uses a strict equality check (address(this).balance == uint256(totalFees)) to verify no player funds remain. This check is inherently fragile because Ethereum allows ETH to be force-sent to any contract via selfdestruct, coinbase rewards, or pre-funded addresses — none of which update totalFees. The fix must remove any dependency on address(this).balance for authorization logic.

Primary fix — Use internal accounting exclusively:

Why this works:

• The function withdraws exactly totalFees worth of ETH — the amount tracked by internal accounting. It never compares against address(this).balance, so force-sent ETH has zero effect on the function's ability to execute.

• The CEI pattern (totalFees = 0 before the external call) prevents reentrancy on the withdrawal itself.

• Any excess ETH in the contract (from selfdestruct or other sources) simply remains as a surplus and doesn't block legitimate fee withdrawals.

Note on selfdestruct deprecation: As of EIP-6780 (Dencun upgrade, March 2024), selfdestruct only transfers ETH without destroying the contract (unless called in the same transaction as creation). However, force-sending ETH is still possible, so the strict equality check remains vulnerable. The internal-accounting approach is the only reliable pattern.

Invariant: After the fix, the following should always hold: address(this).balance >= totalFees + (activePlayerCount * entranceFee). Any surplus is from force-sent ETH and does not affect protocol logic.