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

The player's status is updated before sending eth in `ThePredicter::cancelRegistration` leading to a reentrancy attack

myssteeque

Summary

The function ThePredicter::cancelRegistration contains a reentrancy vulnerability. This occurs because the function makes an external call to send Ether to the caller before updating the caller's status in the playersStatus mapping.

Vulnerability Details

The line (bool success, ) = msg.sender.call{value: entranceFee}(""); sends Ether to msg.sender before the status is updated to Status.Canceled. If msg.sender is a contract, it can execute a fallback or receive function that calls cancelRegistration again, exploiting the vulnerability to withdraw funds multiple times.

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

An attacker can exploit this reentrancy vulnerability to repeatedly call cancelRegistration and drain the contract's funds.

Tools Used

Manual review

Recommendations

Ensure you update the player's status in the playersStatus mapping before making the external call to send Ether, to mitigate this reentrancy attack. Below is the correct implementation of the function to cancelRegistration

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");
- playersStatus[msg.sender] = Status.Canceled;
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.