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

Organiser can claim all the funds after selection period

mattjenkins

Summary

Once the selection period is over, the organiser calls killsRavana on the Dussehra contract. This sends half the funds to the organiser. However there is nothing to stop the organiser (or anyone else for that matter) in calling this function again, and getting the remaining funds sent to the organiser address.

Vulnerability Details

There is no check to prevent the second calling of killsRavana and for the organiser to get sent ALL the funds and leave nothing for the selectRAM to collect. This is demonstrated in the code below. It does not even need the organiser to call killsRavana either.

function test_killRavanaTwice() public participants {
​
vm.assertEq(address(dussehra).balance, 2 ether);
vm.warp(1728691200 + 1);
vm.startPrank(organiser);
choosingRam.selectRamIfNotSelected();
dussehra.killRavana();
vm.stopPrank();
vm.assertEq(address(dussehra).balance, 1 ether);
vm.assertEq(organiser.balance, 1 ether);
​
vm.startPrank(organiser);
dussehra.killRavana();
vm.stopPrank();
vm.assertEq(address(dussehra).balance, 0 ether);
vm.assertEq(organiser.balance, 2 ether);
}

Impact

The organiser can claim all the funds of the protocol once the killRavana period starts:

Tools Used

foundry + manual review

Recommendations

To prevent a double call, we need to add in a check to see if Ravana has already been killed. We can't use the modifier in the contract as it is checking the inverse we need to check. Add the below code to line 77 in Dussehra.sol.

+ if (IsRavanKilled) {
+ revert Dussehra__AlreadyClaimedAmount();
+ }
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.