Beginner FriendlyFoundryNFT
100 EXP
View results
Submission Details
Severity: medium
Valid

killRavana can be called multiple times

Summary

In DOCS is stated that killRavana—Allows users to kill Ravana, and the Organizer will receive half of the total amount collected in the event. This function will only work after 12 October 2024 and before 13 October 2024.
However, users can kill Ravana multiple times.

Vulnerability Details

If we take a look at killRavana function:

function killRavana() public RamIsSelected {
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");
}

We can see that IsRavanKilled is set to true, but we don't check if it is killed after calling the function after first time. This means that killRavana can be called multiple times and the organizer will collect the money.

Impact

Ram not receiving payment.

Proof of Concept

Create the following test case:

function test_killRavanaMultipleTimes() public participants {
vm.warp(1728691200 + 1);
vm.startPrank(organiser);
choosingRam.selectRamIfNotSelected();
vm.stopPrank();
vm.startPrank(player1);
dussehra.killRavana();
vm.stopPrank();
assertEq(dussehra.IsRavanKilled(), true);
assertEq(organiser.balance, 1 ether);
vm.startPrank(player2);
dussehra.killRavana();
vm.stopPrank();
assertEq(organiser.balance, 2 ether);
}

Run forge test --match-test test_killRavanaMultipleTimes
The results are the following:

[PASS] test_killRavanaMultipleTimes() (gas: 410448)
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 7.63ms (537.25µs CPU time)

Tools Used

Manual Review

Recommendations

Consider adding modifier that will check if Ravan is alive.

modifier RavanIsAlive() {
require(!IsRavanKilled, "Ravan is dead!");
_;
}
function killRavana() public RamIsSelected RavanIsAlive {
}
Updates

Lead Judging Commences

bube Lead Judge 12 months ago
Submission Judgement Published
Validated
Assigned finding tags:

isRavanKilled is not checked

Support

FAQs

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