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

Approved players can still register, taking up space in the `ThePredicter::players` array

JJS

Summary

In the ThePredicter::register function, a validation check is performed to ensure a user's state remains Pending, effectively preventing duplicate registrations. However, the function lacks a crucial validation to prevent users who have already been approved from re-registering, potentially leading to unintended consequences in the application logic. This oversight could permit users with an 'Approved' status to re-enter the registration process, which may disrupt the intended flow and data integrity of the system.

Vulnerability Details

function register() public payable {
if (msg.value != entranceFee) {
revert ThePredicter__IncorrectEntranceFee();
}
​
if (block.timestamp > START_TIME - 14400) {
revert ThePredicter__RegistrationIsOver();
}
// Q What if they are already approved?
@> if (playersStatus[msg.sender] == Status.Pending) {
revert ThePredicter__CannotParticipateTwice();
}
​
playersStatus[msg.sender] = Status.Pending;
}

Impact

This oversight permits users with an `Approved` status to register multiple times, potentially monopolizing available slots and thereby limiting opportunities for new users to participate as players. This could lead to an inefficient allocation of resources and diminish the overall accessibility and fairness of the system.

Tools Used

Manual review and test.

Here is a test that can be implemented in ThePredicter.test.sol:

Step by step:

  1. Stranger registers.

  2. Organiser approves them.

  3. Stranger registers again.

function test_approvedPlayersCanStillReigster() public {
// 1.
vm.deal(stranger, 1 ether);
vm.prank(stranger);
thePredicter.register{value: 0.04 ether}();
// 2.
vm.prank(organizer);
thePredicter.approvePlayer(stranger);
// 3.
vm.prank(stranger);
thePredicter.register{value: 0.04 ether}();
}

Recommendations

Change the check to make sure they have not registered and already approved:

```diff
function register() public payable {
if (msg.value != entranceFee) {
revert ThePredicter__IncorrectEntranceFee();
}
​
if (block.timestamp > START_TIME - 14400) {
revert ThePredicter__RegistrationIsOver();
}
// Q What if they are already approved?
- if (playersStatus[msg.sender] == Status.Pending) {
+ if (playersStatus[msg.sender] != Status.Unknown || playersStatus[msg.sender] != Status.Canceled)
revert ThePredicter__CannotParticipateTwice();
}
​
playersStatus[msg.sender] = Status.Pending;
}
```
Updates

Lead Judging Commences

NightHawK Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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