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

[H-3] Re-entrance attack in ThePredicter::cancelRegistration()

ivanonchain

Relevant GitHub Links

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

Summary

A re-entrance attack can be performed in the ThePredicter::cancelRegistration() function resulting in draining all the funds of the contract

Vulnerability Details

The only condition to withdraw the entranceFee is that the status of the registration request must be pending, and the status is changed to canceled only after the funds are sent. Therefore, the sender can repeatedly use the call low level function to perform this action repeatedly to drain the total balance before the status is changed to canceled.

Recommendations

Use the nonReentrant guard modifier that Openzeppelin provide and set the status to canceled before to send the funds.

​
+function cancelRegistration() public nonReentrant {
-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;
}

Impact

The impact is high because a malicious actor can drain the total of funds

Tools Used

Manual review

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.