First Flight #20: The Predicter
First Flight #20
Beginner FriendlyFoundry
100 EXP
View results
Previous Next
Submission Details
Severity: high
Invalid

A single user can enter the contact multiple times with same address.

saurabh_singh

Summary :

A single user can enter the Thepredicter.sol contract multiple times because the revert ThePredicter__CannotParticipateTwice() do not revert when the same user enter twice as intended.

Vulnerability Details :

The check within the ThePredicter::register function solely verifies if the user's status is pending. It only reverts if the same user attempts to register while their status remains pending.

@> if (playersStatus[msg.sender] == Status.Pending) {
@> revert ThePredicter__CannotParticipateTwice();
}

For example, if a user is approved by the organizer, their status changes from pending to approved. Consequently, the above check would fail to prevent the user from registering again.

function approvePlayer(address player) public {
if (msg.sender != organizer) {
revert ThePredicter__UnauthorizedAccess();
}
if (players.length >= 30) {
revert ThePredicter__AllPlacesAreTaken();
}
@> if (playersStatus[player] == Status.Pending) {
@> playersStatus[player] = Status.Approved;
@> players.push(player);
}
}

Impact :

Allowing the same user to enter multiple times can unfairly skew the odds in their favor, compromising the chances of other participants and potentially enabling manipulation of the betting contract.

Proof of code : The below given test will show that one single user can enter the contract multiple times

function testSingleUserCanEnterMoreThanOneTime() public {
address firstplayer = makeAddr("firstplayer");
vm.startPrank(firstplayer);
vm.deal(firstplayer, 1 ether);
thePredicter.register{value: 0.04 ether}();
vm.stopPrank();
vm.startPrank(organizer);
thePredicter.approvePlayer(firstplayer);
vm.stopPrank();
console.log("player enter first time index 0 :-", thePredicter.players(0));
​
vm.startPrank(firstplayer);
vm.deal(firstplayer, 1 ether);
thePredicter.register{value: 0.04 ether}();
vm.stopPrank();
vm.startPrank(organizer);
thePredicter.approvePlayer(firstplayer);
vm.stopPrank();
console.log("same player entered second time index 1 :-", thePredicter.players(1));
​
vm.startPrank(firstplayer);
vm.deal(firstplayer, 1 ether);
thePredicter.register{value: 0.04 ether}();
vm.stopPrank();
vm.startPrank(organizer);
thePredicter.approvePlayer(firstplayer);
vm.stopPrank();
console.log("same player entered third time index 2 :-", thePredicter.players(2));
}

Output of the above test is as follows:

Ran 1 test for test/ThePredicter.test.sol:ThePredicterTest
[PASS] testSingleUserCanEnterMoreThanOneTime() (gas: 170102)
Logs:
player enter first time index 0 :- 0xc755ace5399901C5aa241A9A117C7D4223a947d1
same player entered second time index 1 :- 0xc755ace5399901C5aa241A9A117C7D4223a947d1
same player entered third time index 2 :- 0xc755ace5399901C5aa241A9A117C7D4223a947d1

Clearly we can see that same player enter the player array multiple times

Tools Used : Unit test and Manual review

Recommendations : In the ThePredicter::register function we should verify if the entering user is already listed within the players array.

the added for loop will check if the entering user is in the players array.

function register() public payable {
if (msg.value != entranceFee) {
revert ThePredicter__IncorrectEntranceFee();
}
​
if (block.timestamp > START_TIME - 14400) {
revert ThePredicter__RegistrationIsOver();
}
​
if (playersStatus[msg.sender] == Status.Pending) {
revert ThePredicter__CannotParticipateTwice();
}
+ uint256 playersLength = players.length();
+ for(int i=0 ; i < playersLength ; i++ ){
+ if(players[i] == msg.sender){
+ revert ThePredicter__CannotParticipateTwice();
+ }
+ }
​
playersStatus[msg.sender] = Status.Pending;
}
Updates

Lead Judging Commences

NightHawK Lead Judge 11 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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