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

`Dussehra::killRavana` function can be called twice in row which leads to rewards being denied for selected Ram.

Summary

The Dussehra::killRavana function can be exploited by calling it multiple times within the valid time window, leading to the unintended transfer of funds to the organiser multiple times. This depletes the contract's balance, potentially leaving insufficient funds for the selected Ram to withdraw their rightful share.

Vulnerability Details

// @audit - can be called twice in row
@> function killRavana() public RamIsSelected {
// Oct 11 2024 23:57:49 (GMT)
if (block.timestamp < 1728691069) {
revert Dussehra__MahuratIsNotStart();
}
// Oct 13 2024 00:01:09 (GMT)
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");
}

Dussehra::killRavana function allows to kill Ravana and transfers 50% of all entrance fees to organiser. Problem is that Dussehra::killRavana function can be called twice in row, and second call will again send 50% of all entrance fees to organiser, which means contract will be empty (or almost empty). Then if selected Ram wants to call Dussehra::withdraw function, it will not be able to withdraw because there is no enough funds in Dussehra contract for selected Ram.

  1. Two players mint their Ram NFTs.

  2. After some time organiser calls ChoosingRam::selectRamIfNotSelected function and selects one of player as selected Ram.

  3. Random caller calls Dussehra::killRavana function.

  4. Random caller again calls Dussehra::killRavana function.

  5. Assert that Dussehra contract is empty.

  6. Selected Ram calls Dussehra::withdraw function, and it reverts.

PoC

Place the following test into Dussehra.t.sol.

function test_killRavanaCanBeCalledTwoTimesDenyingRewardForRam() public participants {
vm.warp(1728691200 + 1);
vm.prank(organiser);
choosingRam.selectRamIfNotSelected();
vm.startPrank(makeAddr("randomCaller"));
dussehra.killRavana();
dussehra.killRavana();
vm.stopPrank();
assertTrue(address(dussehra).balance == 0);
vm.prank(choosingRam.selectedRam());
vm.expectRevert("Failed to send money to Ram");
dussehra.withdraw();
}

Impact

If Dussehra::killRavana function is called twice in row by any random caller, selected Ram will not be able to withdraw his reward. In that case all entrance fees will go to organiser.

Tools Used

Manual review

Recommendations

Add modifier RavanNotKilled to Dussehra::killRavana function to prevent that function can be called twice.

+ modifier RavanNotKilled() {
+ require(!IsRavanKilled, "Ravan is already killed");
+ _;
+ }
.
.
- function killRavana() public RamIsSelected {
+ function killRavana() public RamIsSelected RavanNotKilled {
// Oct 11 2024 23:57:49 (GMT)
if (block.timestamp < 1728691069) {
revert Dussehra__MahuratIsNotStart();
}
// Oct 13 2024 00:01:09 (GMT)
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 over 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.