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

Funds are locked in `Dussehra.sol` contract when the `withdraw` function is called.

Summary

When the killRavana function is called in the Dussehra.sol contract, the funds are not divided correctly. This results in a remainder of the division that is not accounted for. This remainder will be locked in the contract and not be able to be withdrawn by the organizer.

Vulnerability Details

Performing totalAmountGivenToRam = (totalAmountByThePeople * 50) / 100; in the Dussehra.sol:killRavana will not always result in a number that is truly 50% of the total amount. This happens when an entry fee, passed to the contracts constructor in Wei is not divisible by 2 when multiplied by the number of participants.

Impact

After the organizer and the selected Ram have received their funds, the remainder of the division will be locked in the contract. This will result in the organizer not being able to withdraw the funds.

Tools Used

Stateless Fuzz Testing.

Proof of Concept:

  1. Dussehra.sol is deployed with an entry fee in wei (fuzzed) that is not divisible by 2 when multiplied by the number of participants.

  2. Enter participants with the fuzzed entrance fee.

  3. Warp to the time when the event is finished

  4. Organizer calls selectRamIfNotSelected and killRavana

  5. The funds are divided and the remainder is locked in the contract.

  6. Organizer recieves half of the funds during the killRavana call.

  7. The selected ram calls 'withdraw` and recieves the other half, while the remainder is locked in the contract.

Steps to Reproduce:

  1. Create a new test file called LockedFundsAfterWithDraw.t.sol

  2. Add the code below to the file.

  3. Run the test with forge test --match-path test/LockedFundsAfterWithDraw.t.sol -vvv

Proof of Code:

Code
// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.13;
import {Test} from "forge-std/Test.sol";
import {Dussehra} from "../src/Dussehra.sol";
import {ChoosingRam} from "../src/ChoosingRam.sol";
import {RamNFT} from "../src/RamNFT.sol";
contract LockedFundsAfterWithDraw is Test {
error Dussehra__NotEqualToEntranceFee();
error Dussehra__AlreadyClaimedAmount();
error ChoosingRam__TimeToBeLikeRamIsNotFinish();
error ChoosingRam__EventIsFinished();
Dussehra public dussehra;
RamNFT public ramNFT;
ChoosingRam public choosingRam;
address public organiser = makeAddr("organiser");
address public player1 = makeAddr("player1");
address public player2 = makeAddr("player2");
address public player3 = makeAddr("player3");
function deploy(uint256 entranceFee) public {
vm.startPrank(organiser);
ramNFT = new RamNFT();
choosingRam = new ChoosingRam(address(ramNFT));
dussehra = new Dussehra(entranceFee, address(choosingRam), address(ramNFT));
ramNFT.setChoosingRamContract(address(choosingRam));
vm.stopPrank();
}
function enterParticipants(uint256 entranceFee) internal {
vm.startPrank(player1);
vm.deal(player1, entranceFee);
dussehra.enterPeopleWhoLikeRam{value: entranceFee}();
vm.stopPrank();
vm.startPrank(player2);
vm.deal(player2, entranceFee);
dussehra.enterPeopleWhoLikeRam{value: entranceFee}();
vm.stopPrank();
vm.startPrank(player3);
vm.deal(player3, entranceFee);
dussehra.enterPeopleWhoLikeRam{value: entranceFee}();
vm.stopPrank();
}
function test_withdraw_locks_funds(uint256 entranceFee) public {
// Set up the contracts with the fuzzed entrance fee
entranceFee = bound(entranceFee, 0.01 ether, 1 ether);
deploy(entranceFee);
// Enter participants with the fuzzed entrance fee
enterParticipants(entranceFee);
// Warp to the time when the event is finished
vm.warp(1728691200 + 1);
// Select Ram as a winner
vm.startPrank(organiser);
choosingRam.selectRamIfNotSelected();
vm.stopPrank();
// Determine the winner
address winner = choosingRam.selectedRam() == player1
? player1
: choosingRam.selectedRam() == player2
? player2
: player3;
vm.startPrank(winner);
dussehra.killRavana();
uint256 RamWinningAmount = dussehra.totalAmountGivenToRam();
// Check the balance of the organiser
assertEq(organiser.balance, RamWinningAmount);
dussehra.withdraw();
vm.stopPrank();
// check the balance of the winner
assertEq(winner.balance, RamWinningAmount);
// check that the balance of the winner and the organiser is the same
assertEq(winner.balance, organiser.balance);
// check that the balance of the contract is 0
assertEq(address(dussehra).balance, 0 ether);
}
}

Recommendations

Performing totalAmountGivenToRam = (totalAmountByThePeople * 50) / 100; is already a more accurate way to calculate the 50% of the total amount. However, the killRavana function should be updated to ensure that after the 50% is calculated, the remainder of contract funds are sent to the organizer. This will prevent any funds from being locked in the contract.

function killRavana() public RamIsSelected {
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;
+ uint256 remainder = totalAmountByThePeople - totalAmountGivenToRam;
- (bool success, ) = organiser.call{value: totalAmountGivenToRam}("");
+ (bool success, ) = organiser.call{value: remainder}("");
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:

Dust

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.