First Flight #2: Puppy Raffle

Summary

The balance check in the PuppyRaffle::withdrawFees function could enable griefer to self-destruct a contract to send ETH to the raffle and blocking withdrawals.

Vulnerability Details

The require statement in the withdrawFees() function checks that the contract's balance is equal to the totalFees and then allow the withdraw. But the malicious user can exploit the function, makes the require statement to revert and blocks the withdrawals.

Impact

The balance check in the withdrawFees function is crucial to prevent unauthorized withdrawals. It's a mechanism to ensure that the contract's balance matches the totalFees before allowing the withdrawal. However, it can be exploited by a malicious actor (griefer) in the following way.
A griefer can deploy a contract that includes a selfdestruct() function. The selfdestruct() function can be set to destroy the contract and send all of its Ether to the PuppyRaffle contract when a certain condition is met.
A griefer can trigger the selfdestruct function, the griefer can call the withdrawFees() function from the PuppyRaffle contract. Since the PuppyRaffle contract checks if the balance equals the totalFees before allowing the withdrawal, the balance check will pass, and the fees will be withdrawn.
When the griefer's contract is destroyed and sends all its Ether to the PuppyRaffle contract, the withdrawFees() function will fail because the contract balance no longer matches the totalFees. This effectively blocks any future withdrawals from the PuppyRaffle contract.
Here is an example of how a griefer contract might look like:

In this contract, the triggerSelfDestruct() function transfers all of the contract's Ether to the PuppyRaffle contract and then destroys the contract. When the PuppyRaffle contract receives the Ether, it will try to withdraw the fees, but the balance check will fail because the contract's balance no longer matches the totalFees. This effectively blocks any future withdrawals from the PuppyRaffle contract.

Tools Used

VS Code, Foundry

Recommendations

To prevent this vulnerability, you can track the balance internally. Instead of relying on address(this).balance to track the balance of the contract, you can maintain an internal variable that is updated whenever Ether is deposited or withdrawn. This way, even if an attacker self-destructs a contract and sends ETH to the raffle, the internal balance will not be affected, and the withdrawal will proceed as expected.