First Flight #2: Puppy Raffle

Summary

Functionality relying on complex array loop operations

Vulnerability Details

Checking duplicates is 0(n^2) complexity

Getting active player index if players index is so large functions like getActivePlayerIndex(...) become too expensive or will always run out of gas and never work

Above function also uses expensive loop operations but could have just used a mapping

Impact

Can lead to gas intensive functions eg when entering raffle, can lead to out of gas for various function and even a DOS if that function runs out of gas and is relied on other functions

Tools Used

Manual Analysis

Recommendations

1. For checking duplicates this must be done offline or make use better looping using mapping to avoid the current complexity 0(n^2) complexity

2. For getActivePlayerIndex(...) instead of looping through players array make use a mapping that has index value if player is active e.g mapping(address => index) addressToIndex; this variable is saved when enterRAffle is called for each address in address[] memory newPlayers

3. Fro _isActivePlayer() instead of looping through players array make use a mapping that has boolean value if player is active e.g mapping(address => bool) isActive; this variable is activated when enterRAffle is called for each address in address[] memory newPlayers