Beginner FriendlyFoundry
100 EXP
View results
Submission Details
Severity: high
Valid

Reentrancy in ThePredicter.cancelRegistration()

Summary

The ThePredicter::cancelRegistration() function doesn't have any mechanism to prevent a reentrancy attack and doesn't follow the Check-effects-interactions pattern

Vulnerability Details

function cancelRegistration() public {
if (playersStatus[msg.sender] == Status.Pending) {
(bool success, ) = msg.sender.call{value: entranceFee}("");
require(success, "Failed to withdraw");
playersStatus[msg.sender] = Status.Canceled;
return;
}
revert ThePredicter__NotEligibleForWithdraw();
}

Impact

If exploited, this vulnerability could allow a malicious contract to drain Ether from the ThePredicter contract, leading to loss of funds for the contract and its users.

Tools Used

Manual Review

Recommendations

To mitigate the reentrancy vulnerability, you should follow the Checks-Effects-Interactions pattern. This pattern suggests that you should make any state changes before calling external contracts or sending Ether.
Here's how you can modify the function:

function cancelRegistration() public {
if (playersStatus[msg.sender] == Status.Pending) {
playersStatus[msg.sender] = Status.Canceled;
(bool success, ) = msg.sender.call{value: entranceFee}("");
require(success, "Failed to withdraw");
return;
}
revert ThePredicter__NotEligibleForWithdraw();
}
Updates

Lead Judging Commences

NightHawK Lead Judge about 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

Reentrancy in cancelRegistration

Reentrancy of ThePredicter::cancelRegistration allows a maliciour user to drain all funds.

Support

FAQs

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