First Flight #2: Puppy Raffle

Summary

The PuppyRaffle#withdrawFees() function is using address(this).balance to check if all the fees for the current round have been collected and therefore the round is over, but just a small amount of ETH added to the player fees cause the function to be DoS.

Vulnerability Details

Using address(this).balance and trust that its value will always be synched with the specific smart contract logic is discouraged. In this case using selfdestruct() to send a small amount of ETH to the PuppyRaffle contract can cause the address(this).balance and the totalFees to always be out of sync causing a DoS on the PuppyRaffle#withdrawFees().

Impact

It is impossible for the owner to collect his fees because the PuppyRaffle#withdrawFees() will always revert.

Tools Used

Manual review.

Recommendations

Use a storage variable (bool or enum) to save the start and end of the raffle instead of relying on the fees collected and the balance.