First Flight #2: Puppy Raffle

Summary

Reentrancy can occur in the PuppyRaffle::refund function

Vulnerability Details

A reentrancy issue can happen in the refund function when the entranceFee is sent to the participant. Basically, we are transferring the fee amount to the msg.sender and after that, we are setting the address of the msg.sender to zero in the players array. This is an issue since we are not using the CEI pattern and not updating the players array before transferring the value.

Impact

This will cause loss of funds for the protocol

Tools Used

Manual Review

Recommendations

The best practice here is to apply correctly the CEI pattern (Checks Effects Interactions): update the players array by removing the player from the array, and do not set its initial address value to zero. Also, here it is better to use .call{value: entranceFee}(""); instead of '.sendValue(entranceFee)', and after that check the status of the call with a require statement. You can also consider adding a nonReentrant modifier.