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 multiple times, allowing the organizer to withdraw 100% of collected fees.

feder

Summary

killRavana lacks a check to ensure that the function cannot be called multiple times. The issue allows any user to call the function at least twice to transfer all collected fees to the organizer, leaving no reward for the user who selected Ram.

Vulnerability Details

The Dussehra protocol is intended for users to be able to mint NFTs for a fee and battle against other NFTs to select Ram. The user who finally selects Ram is entitled to withdraw 50% of the fees collected by the protocol. This can only be achieved after the killRavana function has been called and 50% of the fees have been transferred to the organizer.

The problem with the current implementation lies in the fact that the killRavana function can be called multiple times, sending 50% of the fees collected by the protocol each time. Consequently, after 2 calls, 100% of the fees will be transferred to the organizer and the user who selected Ram will receive no rewards.

function killRavana() public RamIsSelected {
if (block.timestamp < 1728691069) { // 12 Oct >= 12 Oct
revert Dussehra__MahuratIsNotStart();
}
if (block.timestamp > 1728777669) { // 13 Oct <= 13 Oct
revert Dussehra__MahuratIsFinished();
}
IsRavanKilled = true;
uint256 totalAmountByThePeople = WantToBeLikeRam.length * entranceFee;
totalAmountGivenToRam = (totalAmountByThePeople * 50) / 100;
// @audit each time this function is called, 50% of the collected
// fees will be transferred to the organizer.
(bool success, ) = organiser.call{value: totalAmountGivenToRam}("");
require(success, "Failed to send money to organiser");
}
PoC

Add the following test to the test suite. It shows that after Ram is selected and the killRavana function is called twice, the organizer gets 100% of the fees.

Note: In order for this test to pass, the increaseValuesOfParticipants function needs to be fixed by adding isRamSelected = true; after the lines where the variable selectedRam is set to the winner's user address.

function test_withdrawTotalFeesByKillingRavana() public participants {
// Player 1 selects Ram
vm.startPrank(player1);
choosingRam.increaseValuesOfParticipants(0, 0);
choosingRam.increaseValuesOfParticipants(0, 0);
choosingRam.increaseValuesOfParticipants(0, 0);
choosingRam.increaseValuesOfParticipants(0, 0);
choosingRam.increaseValuesOfParticipants(0, 0);
vm.stopPrank();
​
assertEq(choosingRam.selectedRam(), player1);
assertEq(choosingRam.isRamSelected(), true);
​
vm.warp(1728691200 + 1);
assertEq(address(dussehra).balance, 2 ether);
​
uint256 organiserBalanceBefore = organiser.balance;
​
vm.startPrank(organiser);
dussehra.killRavana();
dussehra.killRavana();
vm.stopPrank();
​
assertEq(organiser.balance, organiserBalanceBefore + 2 ether);
}

Impact

  • The organizer could call the killRavana function twice and withdraw 100% of the collected fees, leaving no rewards for the user who selected Ram.

Tools Used

Manual review.

Recommended Mitigation

Prevent the killRavana function from being called multiple times.

function killRavana() public RamIsSelected {
+ require(!IsRavanKilled, "Ravana already killed");
if (block.timestamp < 1728691069) { // 12 Oct >= 12 Oct
revert Dussehra__MahuratIsNotStart();
}
if (block.timestamp > 1728777669) { // 13 Oct <= 13 Oct
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.