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

Reentrancy in cancelRegistration()

Summary

A user can call cancelRegistration() from a contract which will send them ETH. They can then can call the cancelRegistraion() again in loop until the contract is drained.

Vulnerability Details

The below method is prone to reentrancy attack as the status of the player is updated after the ETH is transferred. So the user can call this method in loop and drain the contract.

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

All the users funds will be lost

Tools Used

Vs Code

Recommendations

Update the player status before refunding the funds.

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 11 months 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.