First Flight #2: Puppy Raffle

Summary

The withdrawFees function in the provided code is vulnerable to a loss of all fees due to a condition check on the contract's balance.

Vulnerability Details

The vulnerability arises from the condition check in the withdrawFees function:

If an attacker performs a self-destruct operation on a self-made contract with some ether in it, it will change the address(this).balance value (because contract won’t be able to refuse money), making the equality check never true. As a result, the fees collected in the contract will be locked forever.

Impact

The impact of this vulnerability is the loss of all fees collected in the contract. Since the distribution of the prize is calculated based solely on the product of the number of players and the entrance fee, 20% of any additional fees sent to the contract will remain locked, as the condition check in the withdrawFees function will never pass.

It does not impact the selectWinner() function, so 80% of funds will always be distributed to each winner, same for the NFT.

Tools Used

Manual review.

Recommendations

To mitigate this vulnerability, consider implementing the following measures:

1. Purpose of this check is to know if there are currently player active. So check players.length == 0 instead of using address(this).balance.

2. Alternatively, implement a lock to know when the raffle is running or not.