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

Reentrancy attack in `ThePredicter::cancelRegistration` allows attacker to drain contract balance

pob001

Relevant GitHub Links

https://github.com/Cyfrin/2024-07-the-predicter/blob/main/src/ThePredicter.sol#L64

Summary

The ThePredicter::cancelRegistration function does not follow CEIFREI-PI principles and, as a result, enables participants to drain the contract balance.

In the ThePredicter::cancelRegistration function, we first make an external call to the msg.sender address, and only after making that external call, we update the playersStatus array.

A player who has entered the prediction could have a fallback/receive function that calls the ThePredicter::cancelRegistration function again and claim another refund. They could continue to cycle this until the contract balance is drained.

Impact

All fees paid by players could be stolen by a malicious participant.

Recommendations

To fix this, we should have the ThePredicter::cancelRegistration function update the playersStatus array before making the external call.

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 over 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.