Beginner FriendlyFoundryNFT
100 EXP
View results
Submission Details
Severity: low
Valid

Unexpected behaviour if user depends solely on the function description

Summary

The function returns a value that does not match its description under certain situation.

Vulnerability Details

The getActivePlayerIndex function are designed to return the index of an item inside an array. However, the zero value is returned when there is no corresponding items in the array. Consider the first user that leverage the enterRaffle function for registration with the first player of the parameter is addr. Once the user no longer wants to participants in the contest and try to refund, after checking the getActivePlayerIndex function and get zero value. The user can not determine whether it means there is no registration record or the index of addr is 0. Due to the logic flaw, the first registration account might lose the right to refund.

Impact

If an user rely on the return value of getActivePlayerIndex to determine whether it can refund. The misleading zero value might cause an user fail to refund properly. This might be a controversy between the project team and the end user.

Tools Used

Manual review

Recommendations

There is an unused internal function in the contract, that is, _isActivePlayer. This function can be leveraged to determine whether the player is registered or not. After filtering the registration at the very beginning, the function returns the real index of the item.

function getActivePlayerIndex(address player) external view returns (uint256) {
bool active = _isActivePlayer();
require(active, "player not exist");
for (uint256 i = 0; i < players.length; i++) {
if (players[i] == player) {
return i;
}
}
return 0;
}
Updates

Lead Judging Commences

Hamiltonite Lead Judge over 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

getActivePlayerIndex can say a player is both entered at slot 0 and inactive

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.