First Flight #2: Puppy Raffle

Summary

Every time a player calls enterRaffle, enterRaffle iterates through the entire players array checking for duplicates, rather than just comparing the new entries against the existing entries. For example, let's say 2 users A and B have called enterRaffle, each entering with their own address. When B called enterRaffle, it checked that A and B were unique. Now user C comes along and makes one entry with his address. enterRaffle will check C against A and C against B, but it will also check A against B even though that is not necessary since it has already been done.

Vulnerability Details

Impact

You are wasting gas iterating through people who you have already checked for duplicates. Loops are very expensive.

Tools Used

Manual review

Recommendations

You could fix this by saving the length of the players array before pushing the new players into a variable and then having the loop start with at that index as follows:
Add this at the beginning of the function:

Then in the check for duplicates have the inner loop start from the index numberOfExistingPlayers (since the length will be one longer than the length so players[numberOfExistingPlayers] is equal to the index of the first new player):

But I think an even better way to check for duplicates is using a mapping as I suggested in a different finding