Summary
The entry point into the protocol is Dussehra::enterPeopleWhoLikeRam
.
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);
}
The Dussehra::killRavana()
function calculates the distribution of fees between RamNFT::organiser
and one lucky entrant
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;
(bool success, ) = organiser.call{value: totalAmountGivenToRam}("");
require(success, "Failed to send money to organiser");
}
Without this function, there is no other way for the winning entrant ChoosingRam::selectedRam
or RamNFT::organiser
to collect fees
This function can only be called within a window of time dictated by fixed timestamps
Other functions also have important timestamp constraints
ChoosingRam::selectRamIfNotSelected()
function selectRamIfNotSelected() public RamIsNotSelected OnlyOrganiser {
if (block.timestamp < 1728691200) {
revert ChoosingRam__TimeToBeLikeRamIsNotFinish();
}
if (block.timestamp > 1728777600) {
revert ChoosingRam__EventIsFinished();
}
...
}
ChoosingRam::increaseValuesOfParticipants()
function increaseValuesOfParticipants(uint256 tokenIdOfChallenger, uint256 tokenIdOfAnyPerticipent)
public
RamIsNotSelected
{
...
if (block.timestamp > 1728691200) {
revert ChoosingRam__TimeToBeLikeRamFinish();
}
...
}
The issue is that the protocol runs for a period of time. After funds have been disbursed to RamNFT::organiser
and withdrawn by ChoosingRam::selectedRam
, the protocol is "finished". Despite the ending, the story continues and users are still able to enter the protocol paying the entrance fee. These late entrance fees will remain stuck in the contract.
Vulnerability Details
This foundry test highlights the oversight:
pragma solidity ^0.8.13;
import {Test, console} from "forge-std/Test.sol";
import {Dussehra} from "../../src/Dussehra.sol";
import {ChoosingRam} from "../../src/ChoosingRam.sol";
import {RamNFT} from "../../src/RamNFT.sol";
contract CounterTest is Test {
error Dussehra__MahuratIsFinished();
Dussehra public dussehra;
RamNFT public ramNFT;
ChoosingRam public choosingRam;
address public organiser = makeAddr("organiser");
uint256 public constant PICK_A_NUMBER = 5;
address[] players = new address[](PICK_A_NUMBER);
function setUp() public {
vm.startPrank(organiser);
ramNFT = new RamNFT();
choosingRam = new ChoosingRam(address(ramNFT));
dussehra = new Dussehra(1 ether, address(choosingRam), address(ramNFT));
ramNFT.setChoosingRamContract(address(choosingRam));
vm.stopPrank();
}
modifier participants() {
for (uint256 i =0; i< PICK_A_NUMBER; i++) {
string memory stringNumber = vm.toString(i);
players[i] = makeAddr(stringNumber);
vm.deal(players[i], 1 ether);
vm.startPrank(players[i]);
dussehra.enterPeopleWhoLikeRam{value: 1 ether}();
vm.stopPrank();
}
_;
}
function test_joinAfterComplete() public participants {
vm.warp(1728691200 + 1);
vm.startPrank(organiser);
choosingRam.selectRamIfNotSelected();
address selected = choosingRam.selectedRam();
console.log("selected: ", selected);
vm.stopPrank();
uint256 contractBalance = address(dussehra).balance;
console.log("contractBalance start: ", contractBalance);
vm.startPrank(address(selected));
dussehra.killRavana();
vm.stopPrank();
contractBalance = address(dussehra).balance;
console.log("contractBalance after kill: ", contractBalance);
uint256 ramwinningAmount = dussehra.totalAmountGivenToRam();
console.log("ramwinningAmount before withdraw: ", ramwinningAmount);
vm.startPrank(address(selected));
dussehra.withdraw();
vm.stopPrank();
contractBalance = address(dussehra).balance;
console.log("contractBalance after withdraw: ", contractBalance);
ramwinningAmount = dussehra.totalAmountGivenToRam();
console.log("ramwinningAmount after withdraw: ", ramwinningAmount);
vm.warp(1728777669 + 1);
address[] memory fools = new address[](5);
for (uint256 i =0; i< 5; i++) {
string memory stringNumber = vm.toString(i);
string memory base = "fools";
string memory fullString = string(abi.encodePacked(base, stringNumber));
fools[i] = makeAddr(fullString);
vm.deal(fools[i], 1 ether);
vm.startPrank(fools[i]);
dussehra.enterPeopleWhoLikeRam{value: 1 ether}();
vm.stopPrank();
}
contractBalance = address(dussehra).balance;
console.log("contractBalance after fools: ", contractBalance);
ramwinningAmount = dussehra.totalAmountGivenToRam();
console.log("ramwinningAmount after fools: ", ramwinningAmount);
vm.expectRevert(abi.encodeWithSelector(Dussehra__MahuratIsFinished.selector));
vm.startPrank(address(selected));
dussehra.killRavana();
vm.stopPrank();
}
}
The test runs through setting up the contracts, entering participants, declaring a winner, and disbursing funds
After warping to the period where the "contest is finished" found in Dussehra::killRavana()
(1728777669), new participants are still able to enter the protocol
Dussehra::killRavana()
cannot be called at this point, and as we learned, this function is the only way to calculate who gets how much
The Dussehra::entranceFee
paid by all the new entrants will remain stuck in this contract
Impact
This is a high risk vulnerability that leaves funds stuck in the contract.
The protocol uses a function Dussehra::killRavana()
, to calculate the disbursement of fees. The function can only be called within a specific time frame. At the end of this time frame, the protocol is considered to be finished. But there is no mechanism in place preventing new entrants.
The protocol does not stop new participants from joining, even though there are timestamp constraints placed on every other function. Dussehra::enterPeopleWhoLikeRam
needs to implement time constraints as well. If left as is, it will lead to a loss of funds resulting from a mistake that can easily be made by unaware, hopeful participants.
Tools Used
Recommendations
The change needed for Dussehra::enterPeopleWhoLikeRam
is simple. Just add a timestamp constraint.
function enterPeopleWhoLikeRam() public payable {
+ if (block.timestamp > 1728691200) {
+ revert ChoosingRam__TimeToBeLikeRamIsFinished();
+ }
...
}