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

Incorrect timestamps in `Dussehra.sol:killRavana` means ravana can be killed after the allowed time.

Summary

Incorrect timestamps in Dussehra.sol::killRavana checks means ravana can be killed after the allowed time.

Vulnerability Details

The check for if its too early is also wrong as it can be called after Fri Oct 11 2024 23:57:49 GMT+0000 instead of Oct 12 00:00:00. However, because of timestamp checks in ChoosingRam.sol::selectRamIfNotSelected, the ChoosingRam__TimeToBeLikeRamIsNotFinish revert will never hit as ram cannot be selected organically and must be randomly picked by the organiser with ChoosingRam.sol::selectRamIfNotSelected. However, the time check for too late means that you can kill ravana 69 seconds after the allowed time until Sun Oct 13 2024 00:01:09 GMT+0000.

Proof of code

function test_killRavanaLate() public participants {
vm.warp(1728691200 + 1);
vm.startPrank(organiser);
choosingRam.selectRamIfNotSelected();
vm.stopPrank();
vm.warp(1728777669); // set time to Sun Oct 13 2024 00:01:09 GMT+0000
vm.startPrank(choosingRam.selectedRam());
dussehra.killRavana();
vm.stopPrank();
assertEq(dussehra.IsRavanKilled(), true);
}

Impact

Ravana can be killed after Sun Oct 13 2024 00:00:00 GMT+0000 at any time up to Sun Oct 13 2024 00:01:09 GMT+0000.

Tools Used

Manual review and https://www.unixtimestamp.com/ for timestamp calculations

Recommendations

  1. Simple solution is to change the timestamps to the corect times in the ChoosingRam.sol::selectRamIfNotSelected.

  2. Best practice solution: For both Dussehra.sol and ChoosingRam.sol, store start and end times and state variables. Or ideally, one of the contracts, for example, Dussehra.sol will have a startTime and endTime variables that can be accessed by ChoosingRam.sol so that it is certain that the timestamps are consistent amongst the different contracts.

+ uint256 private muhuratStartTimeStamp = 1728691200;
+ function getMuhuratStartTimeStamp() public view returns (uint256) {
+ return muhuratStartTimeStamp;
+ }
+ uint256 private muhuratEndTimeStamp = 1728777600;
+ function getMuhuratEndTimeStamp() public view returns (uint256) {
+ return muhuratEndTimeStamp;
+ }
function killRavana() public RamIsSelected {
// @audit hardcoded timestamps are incorrect by a few minutes
- if (block.timestamp < 1728691069) {
+ if (block.timestamp < muhuratStartTimeStamp ) {
revert Dussehra__MahuratIsNotStart();
}
- if (block.timestamp > 1728777669) {
+ if (block.timestamp > muhuratEndTimeStamp ) {
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");
}
  1. It would be nicer if the docs were a bit more explicit because it is a little ambiguous. E.g instead of "This function will only work after 12 October 2024 and before 13 October 2024.", you could write "This function will only work from 12 October 00:00:00 2024 (GMT) until 13 October 00:00:00 2024 (GMT)." or whatever time period and timezone you intend.

Updates

Lead Judging Commences

bube Lead Judge about 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

Incorrect timestamp

Support

FAQs

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