First Flight #2: Puppy Raffle

Summary

The getActivePlayerIndex function always returns 0 if the player address is not found in the players array, however 0 is a valid index.

Vulnerability Details

In the current code the id 0 refers to the first player to enter the raffle. so it is a bug to return 0 if a player is is not found in the array. It is not exposing any attack vectors.

Impact

It is low impact because the user or a dapp will just be under the mistaken impression that the address is registered in the raffle, but it has no effect on the actual raffle nor does it effect any logic in the codebase.

However it may cause the user to send transactions which will revert and waste gas, and will lead to a confusing user experience.

Tools Used

Manual Review

Recommendations

The issue is that the function must return an unsigned integer, but all unsigned integers are 'technically' valid indices.It is recommended to use a tuple return value (uint,boolean), as in
'function getActivePlayerIndex(address player) external view returns (uint256,bool)'
Then, it is safe to to return (0,false) in the event that a player index is not found.
It is technically ok to use a very large number which can not practically be reached without gas issues as a flag, but that would lead to confusing codebase and confusing interactions with it.