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

Post-Ram Selection Entrance Leading to Locked Funds

Summary

A vulnerability exists in the Dussehra smart contract, allowing participants to pay the entrance fee and enter the festival even after Ram has been selected. This results in Ether being locked in the contract with no way of retrieving it. Additionally, it is unnecessary for users to enter the festival and pay the fee after the event has ended.

Vulnerability Details

The enterPeopleWhoLikeRam function does not check if Ram has already been selected before allowing participants to pay the entrance fee. As a result, participants can continue to pay the fee and enter the festival even after Ram has been selected, leading to locked funds in the contract.

function enterPeopleWhoLikeRam() public payable {
if (msg.value != entranceFee) {
revert Dussehra__NotEqualToEntranceFee();
}
if (peopleLikeRam[msg.sender] == true){
revert Dussehra__AlreadyPresent();
}
peopleLikeRam[msg.sender] = true;
WantToBeLikeRam.push(msg.sender);
ramNFT.mintRamNFT(msg.sender);
emit PeopleWhoLikeRamIsEntered(msg.sender);
}

Likelihood of Occurrence

This vulnerability is highly likely to occur because users could unknowingly pay the entrance fee after the Ram is selected, leading to financial loss. The locked funds could not be retrieved by either the organizer or Ram, resulting in a permanent loss of the paid Ether.

Proof of Concept

The PoC demonstrates the vulnerability by simulating the entry of a participant (player3) attempting to pay the entrance fee after the Ram has already been selected. This results in the participant's Ether becoming locked within the contract, as shown by the test case.

function test_ifFundsWillBeLocked() external {
vm.startPrank(player1);
vm.deal(player1, 1 ether);
dussehra.enterPeopleWhoLikeRam{value: 1 ether}();
vm.stopPrank();
vm.startPrank(player2);
vm.deal(player2, 1 ether);
dussehra.enterPeopleWhoLikeRam{value: 1 ether}();
vm.stopPrank();
vm.warp(1728691200);
vm.startPrank(organiser);
choosingRam.selectRamIfNotSelected();
dussehra.killRavana();
vm.stopPrank();
assertEq(organiser.balance, 1 ether);
vm.startPrank(player3);
vm.deal(player3, 1 ether);
dussehra.enterPeopleWhoLikeRam{value: 1 ether}();
vm.stopPrank();
vm.startPrank(choosingRam.selectedRam());
dussehra.withdraw();
vm.stopPrank();
assertEq(1 ether, choosingRam.selectedRam().balance);
assertEq(1 ether, address(dussehra).balance);
}

After running the test we can confirm the vulnerability:

forge test --match-test test_ifFundsWillBeLocked -vvv
[⠊] Compiling...
No files changed, compilation skipped
Ran 1 test for test/Dussehra.t.sol:CounterTest
[PASS] test_ifFundsWillBeLocked() (gas: 543489)
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 1.43ms (267.42µs CPU time)
Ran 1 test suite in 2.98ms (1.43ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)

Impact

The vulnerability in the Dussehra smart contract, which allows participants to pay the entrance fee and enter the festival even after the Ram has surely been selected, has several significant impacts:

  1. Locked Funds: Ether paid by participants gets permanently locked in the contract. Neither the organizer nor Ram* can retrieve these funds, leading to a financial loss for participants.

  2. Participant Misguidance: Users might unknowingly pay the entrance fee after the event has ended, believing they can still participate and become the Ram. This results in unnecessary expenditure and frustration among participants.

  3. Contract Integrity: Allowing payments after the event's conclusion undermines the contract's integrity. It indicates a lack of proper checks and balances within the contract's logic, reducing trust in the smart contract's functionality.

  4. Potential Exploitation: If the killRavana function is called repeatedly, it will continuously recalculate the funds to send. Furthermore, if the withdraw function is called, funds can only be "retrieved" after waiting for more users to enter the event post-conclusion, which makes no sense and allows for potential fund mismanagement. Ram can only take money from the contract once the funds inside equal what should be sent to Ram (would be considered as stealing). However, since the event is over, any new deposits would be unnecessary and constitute an exploit of the contract logic.

Tools Used

  1. Manual Code Review

  2. Foundry

Recommendations

To mitigate this vulnerability, update the enterPeopleWhoLikeRam function to include a check for RamIsNotSelected. This ensures that participants cannot pay the entrance fee and enter the festival after the Ram has been selected, thus preventing locked funds and maintaining the integrity of the event.

+ modifier RamIsNotSelected() {
+ require(!choosingRamContract.isRamSelected(), "Ram is selected!");
+ _;
+ }
+ function enterPeopleWhoLikeRam() public payable RamIsNotSelected {
if (msg.value != entranceFee) {
revert Dussehra__NotEqualToEntranceFee();
}
if (peopleLikeRam[msg.sender] == true){
revert Dussehra__AlreadyPresent();
}
peopleLikeRam[msg.sender] = true;
WantToBeLikeRam.push(msg.sender);
ramNFT.mintRamNFT(msg.sender);
emit PeopleWhoLikeRamIsEntered(msg.sender);
}

After updating the code we run the test again, and it fails:

forge test --match-test test_ifFundsWillBeLocked -vv
[⠊] Compiling...
No files changed, compilation skipped
Ran 1 test for test/Dussehra.t.sol:CounterTest
[FAIL. Reason: revert: Ram is selected!] test_ifFundsWillBeLocked() (gas: 410085)
Suite result: FAILED. 0 passed; 1 failed; 0 skipped; finished in 1.35ms (239.10µs CPU time)
Ran 1 test suite in 2.83ms (1.35ms CPU time): 0 tests passed, 1 failed, 0 skipped (1 total tests)
Failing tests:
Encountered 1 failing test in test/Dussehra.t.sol:CounterTest
[FAIL. Reason: revert: Ram is selected!] test_ifFundsWillBeLocked() (gas: 410085)
Encountered a total of 1 failing tests, 0 tests succeeded

By adding the check for RamIsNotSelected the contract will prevent participants from entering the festival and paying the entrance fee after the event has ended, thus avoiding locked funds.

Conclusion

By implementing this check, the contract ensures that participants cannot enter the event after the Ram has been selected, thereby preventing locked funds and maintaining the integrity of the festival.

Updates

Lead Judging Commences

bube Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

Invalid - enter people after event or after Ram is selected

It is the user's responsibility to check the date of the event.

Support

FAQs

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

Give us feedback!