First Flight #2: Puppy Raffle

Summary

Raffle refund does not decrease array length.

Vulnerability Details

The refund function changes the value at the player's index to address(0). This is not ideal because the selectWinner function calculates fees based on the length of the players array. So if people join and then refund, the number of players will be n, but the array length will be n + x. This will cause miscalculations when trying to send funds to the winner or to the feeAddress.

Impact

High. Can break the entire functionality of the raffle as calculations will be off.

Tools Used

n/a

Recommendations

Do not use players.length to keep track of the number of registered players. Use a different variable that you can increment/decrement easily as people join or leave. To continue using players.length, one would have to remove a player by deleting his entry in the array and shrinking the entire array to fill his position. This is an O(n) operation, which can be very expensive.