Summary
Dussehra::killRavana
can be called twice and by anyone, and is subject to reentrancy attack
Vulnerability Details
Dussehra::killRavana
can be called twice and by anyone, This allows any user or a malicious organizer to call the function again after killin Ravana the first time, leading to the organizer receiving the entire pot and for Ram to lose acces to their rewards.
There's also another vulnerability present, which is the reentrancy attack that can be done by a malicious orrganizer as the Code doesn't follow CEI best practices
Impact
This vulnerability undermines the integrity of the event, as participants might lose trust in the fairness of the system. Potential participants may decide not to join the event, leaving Ravana undefeated and the event unsuccessful.
function test_killRavanaTwice() public participants {
address malicious_user = makeAddr("malicious_user");
vm.warp(1728691200 + 1);
vm.startPrank(organiser);
choosingRam.selectRamIfNotSelected();
vm.stopPrank();
assertEq(choosingRam.selectedRam(), player2);
vm.startPrank(player2);
dussehra.killRavana();
vm.stopPrank();
vm.startPrank(malicious_user);
dussehra.killRavana();
vm.stopPrank();
vm.startPrank(player2);
vm.expectRevert("Failed to send money to Ram");
dussehra.withdraw();
vm.stopPrank();
uint256 RamwinningAmount = dussehra.totalAmountGivenToRam();
assertEq(organiser.balance, 2 * RamwinningAmount);
}
Tools Used
Foundry
Recommendations
Add RavanKilled Modifier this serves as a Reentrancy guard:
- function killRavana() public RamIsSelected {
+ function killRavana() public RamIsSelected RavanKilled {
if (block.timestamp < 1728691069) {
revert Dussehra__MahuratIsNotStart();
}
if (block.timestamp > 1728777669) {
revert Dussehra__MahuratIsFinished();
}
IsRavanKilled = true;
uint256 totalAmountByThePeople = WantToBeLikeRam.length * entranceFee;
totalAmountGivenToRam = (totalAmountByThePeople * 50) / 100;
(bool success, ) = organiser.call{value: totalAmountGivenToRam}("");
require(success, "Failed to send money to organiser");
}