First Flight #2: Puppy Raffle

Summary

This double for loop in enterRaffle() is vulnerable to a DOS attack.

Vulnerability Details

A malicious user can trivially add many (distinct) addresses repeatedly until it costs too much gas for the nested for loop to execute its checks, preventing others from entering the raffle.

Impact

Any that time someone tries to enter into the raffle once this point has been reached, the function will revert. This means one malicious user could generate many keys and pass those addresses in until noone can enter. they can exploit this to effectively ensure they will win the raffle.

Additionally the users who try to enter once this DOS has occurred will lose all of their gas paid in their transaction, and will get an unexpected out of gas error.

Recommendations

either

1. put an upperbound on the possible entries to avoid unexpected reversions and wasted gas for failed raffle entries, or use the following pattern to reduce the duplicate check to O(1)

mapping(address => bool) public isPlayerEntered
address[] public players;

It will be a duplicate if isPlayerEntered[players[i]] is true.